From 139a6c3f33285405ce53a4fe26aa5529cdf6ebd9 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Mon, 14 Sep 2026 19:07:14 -0700 Subject: [PATCH 1/4] feat(cleanup): add bounded manual retention runs --- .../api/cron/cleanup-soft-deletes/route.ts | 20 +- apps/sim/app/api/logs/cleanup/route.test.ts | 75 ++++ apps/sim/app/api/logs/cleanup/route.ts | 20 +- apps/sim/background/cleanup-bounded.test.ts | 211 +++++++++++ apps/sim/background/cleanup-logs-bounded.ts | 157 ++++++++ apps/sim/background/cleanup-logs.test.ts | 2 +- apps/sim/background/cleanup-logs.ts | 195 +++++----- .../cleanup-soft-deletes-bounded.ts | 352 ++++++++++++++++++ .../background/cleanup-soft-deletes.test.ts | 7 +- apps/sim/background/cleanup-soft-deletes.ts | 81 ++-- apps/sim/lib/api/contracts/cleanup.test.ts | 33 ++ apps/sim/lib/api/contracts/cleanup.ts | 103 +++++ .../lib/billing/cleanup-dispatcher.test.ts | 122 +++++- apps/sim/lib/billing/cleanup-dispatcher.ts | 226 +++++++---- .../lib/billing/core/billing-attribution.ts | 9 +- apps/sim/lib/billing/storage/context.ts | 10 +- apps/sim/lib/cleanup/bounded-cleanup.md | 75 ++++ apps/sim/lib/cleanup/bounded-delete.ts | 43 +++ .../cleanup/bounded-large-value-metadata.ts | 82 ++++ apps/sim/lib/cleanup/bounded-runner.ts | 49 +++ apps/sim/lib/cleanup/bounded-storage.ts | 41 ++ apps/sim/lib/cleanup/bounded-types.ts | 53 +++ .../cleanup/bounded.postgres.integration.ts | 199 ++++++++++ apps/sim/lib/cleanup/bounded.test.ts | 114 ++++++ apps/sim/lib/cleanup/bounded.ts | 145 ++++++++ apps/sim/lib/cleanup/chat-cleanup.ts | 121 ++++-- apps/sim/lib/cleanup/queue.ts | 2 + .../sim/lib/cleanup/vitest.postgres.config.ts | 13 + .../payloads/large-value-metadata.ts | 95 +++-- apps/sim/lib/knowledge/documents/service.ts | 74 ++-- .../workspace/workspace-file-manager.ts | 7 +- 31 files changed, 2432 insertions(+), 304 deletions(-) create mode 100644 apps/sim/app/api/logs/cleanup/route.test.ts create mode 100644 apps/sim/background/cleanup-bounded.test.ts create mode 100644 apps/sim/background/cleanup-logs-bounded.ts create mode 100644 apps/sim/background/cleanup-soft-deletes-bounded.ts create mode 100644 apps/sim/lib/api/contracts/cleanup.test.ts create mode 100644 apps/sim/lib/api/contracts/cleanup.ts create mode 100644 apps/sim/lib/cleanup/bounded-cleanup.md create mode 100644 apps/sim/lib/cleanup/bounded-delete.ts create mode 100644 apps/sim/lib/cleanup/bounded-large-value-metadata.ts create mode 100644 apps/sim/lib/cleanup/bounded-runner.ts create mode 100644 apps/sim/lib/cleanup/bounded-storage.ts create mode 100644 apps/sim/lib/cleanup/bounded-types.ts create mode 100644 apps/sim/lib/cleanup/bounded.postgres.integration.ts create mode 100644 apps/sim/lib/cleanup/bounded.test.ts create mode 100644 apps/sim/lib/cleanup/bounded.ts create mode 100644 apps/sim/lib/cleanup/queue.ts create mode 100644 apps/sim/lib/cleanup/vitest.postgres.config.ts diff --git a/apps/sim/app/api/cron/cleanup-soft-deletes/route.ts b/apps/sim/app/api/cron/cleanup-soft-deletes/route.ts index 1df6df035e3..520db937568 100644 --- a/apps/sim/app/api/cron/cleanup-soft-deletes/route.ts +++ b/apps/sim/app/api/cron/cleanup-soft-deletes/route.ts @@ -1,18 +1,36 @@ import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' +import { softDeletesCleanupContract } from '@/lib/api/contracts/cleanup' +import { parseRequest } from '@/lib/api/server/validation' import { verifyCronAuth } from '@/lib/auth/internal' -import { dispatchCleanupJobs } from '@/lib/billing/cleanup-dispatcher' +import { dispatchBoundedCleanup, dispatchCleanupJobs } from '@/lib/billing/cleanup-dispatcher' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' export const dynamic = 'force-dynamic' const logger = createLogger('SoftDeleteCleanupAPI') +/** Cron-secret maintenance protocol is global; workspace principal authorization does not apply. */ export const GET = withRouteHandler(async (request: NextRequest) => { try { const authError = verifyCronAuth(request, 'soft-delete cleanup') if (authError) return authError + const parsed = await parseRequest( + softDeletesCleanupContract, + request, + {}, + { + rejectDuplicateQueryValues: true, + rejectBlankQueryValues: true, + } + ) + if (!parsed.success) return parsed.response + if (parsed.data.query) { + const result = await dispatchBoundedCleanup('cleanup-soft-deletes', parsed.data.query) + return NextResponse.json(result, { status: 202 }) + } + const result = await dispatchCleanupJobs('cleanup-soft-deletes') logger.info('Soft-delete cleanup jobs dispatched', result) diff --git a/apps/sim/app/api/logs/cleanup/route.test.ts b/apps/sim/app/api/logs/cleanup/route.test.ts new file mode 100644 index 00000000000..1e05c6f77d4 --- /dev/null +++ b/apps/sim/app/api/logs/cleanup/route.test.ts @@ -0,0 +1,75 @@ +import { createMockRequest } from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { auth, bounded, scheduled } = vi.hoisted(() => ({ + auth: vi.fn(), + bounded: vi.fn(), + scheduled: vi.fn(), +})) +vi.mock('@/lib/auth/internal', () => ({ verifyCronAuth: auth })) +vi.mock('@/lib/billing/cleanup-dispatcher', () => ({ + dispatchBoundedCleanup: bounded, + dispatchCleanupJobs: scheduled, +})) + +import { GET as softDeletes } from '@/app/api/cron/cleanup-soft-deletes/route' +import { GET as logs } from '@/app/api/logs/cleanup/route' + +for (const [path, GET, type, limit] of [ + ['/api/logs/cleanup', logs, 'cleanup-logs', 'workflowLogs'], + ['/api/cron/cleanup-soft-deletes', softDeletes, 'cleanup-soft-deletes', 'workflows'], +] as const) { + describe(path, () => { + beforeEach(() => { + vi.clearAllMocks() + auth.mockReturnValue(null) + bounded.mockResolvedValue({ runId: 'run-one', mode: 'bounded' }) + scheduled.mockResolvedValue({ + jobIds: ['batch-one'], + jobCount: 1, + chunkCount: 2, + workspaceCount: 3, + }) + }) + const request = (query = '') => + createMockRequest('GET', undefined, {}, `http://localhost:3000${path}${query}`) + it('authenticates before parsing invalid limits', async () => { + auth.mockReturnValue(new Response(null, { status: 401 })) + expect((await GET(request('?unknown=1'))).status).toBe(401) + expect(bounded).not.toHaveBeenCalled() + expect(scheduled).not.toHaveBeenCalled() + }) + it('keeps no-parameter scheduled dispatch unchanged', async () => { + const response = await GET(request()) + expect(response.status).toBe(200) + expect(scheduled).toHaveBeenCalledWith(type) + expect(bounded).not.toHaveBeenCalled() + }) + it('accepts one bounded run', async () => { + const response = await GET(request(`?${limit}=2&requestId=wave-1&dryRun=true`)) + expect(response.status).toBe(202) + expect(bounded).toHaveBeenCalledWith( + type, + expect.objectContaining({ + limits: expect.objectContaining({ [limit]: 2 }), + requestId: 'wave-1', + dryRun: true, + batchSize: 25, + }) + ) + expect(scheduled).not.toHaveBeenCalled() + }) + it.each(['?dryRun=true', '?unknown=1', '?batchSize=3', `?${limit}=2&${limit}=3&requestId=r`])( + 'rejects invalid query %s', + async (query) => { + expect((await GET(request(query))).status).toBe(400) + expect(bounded).not.toHaveBeenCalled() + expect(scheduled).not.toHaveBeenCalled() + } + ) + it('reports a dispatch failure', async () => { + bounded.mockRejectedValue(new Error('Trigger unavailable')) + expect((await GET(request(`?${limit}=2&requestId=r`))).status).toBe(500) + }) + }) +} diff --git a/apps/sim/app/api/logs/cleanup/route.ts b/apps/sim/app/api/logs/cleanup/route.ts index 7891a763bc6..7b8d4259cb4 100644 --- a/apps/sim/app/api/logs/cleanup/route.ts +++ b/apps/sim/app/api/logs/cleanup/route.ts @@ -1,18 +1,36 @@ import { createLogger } from '@sim/logger' import { type NextRequest, NextResponse } from 'next/server' +import { logsCleanupContract } from '@/lib/api/contracts/cleanup' +import { parseRequest } from '@/lib/api/server/validation' import { verifyCronAuth } from '@/lib/auth/internal' -import { dispatchCleanupJobs } from '@/lib/billing/cleanup-dispatcher' +import { dispatchBoundedCleanup, dispatchCleanupJobs } from '@/lib/billing/cleanup-dispatcher' import { withRouteHandler } from '@/lib/core/utils/with-route-handler' export const dynamic = 'force-dynamic' const logger = createLogger('LogsCleanupAPI') +/** Cron-secret maintenance protocol is global; workspace principal authorization does not apply. */ export const GET = withRouteHandler(async (request: NextRequest) => { try { const authError = verifyCronAuth(request, 'logs cleanup') if (authError) return authError + const parsed = await parseRequest( + logsCleanupContract, + request, + {}, + { + rejectDuplicateQueryValues: true, + rejectBlankQueryValues: true, + } + ) + if (!parsed.success) return parsed.response + if (parsed.data.query) { + const result = await dispatchBoundedCleanup('cleanup-logs', parsed.data.query) + return NextResponse.json(result, { status: 202 }) + } + const result = await dispatchCleanupJobs('cleanup-logs') logger.info('Log cleanup jobs dispatched', result) diff --git a/apps/sim/background/cleanup-bounded.test.ts b/apps/sim/background/cleanup-bounded.test.ts new file mode 100644 index 00000000000..86651637c8f --- /dev/null +++ b/apps/sim/background/cleanup-bounded.test.ts @@ -0,0 +1,211 @@ +import { + dbChainMock, + dbChainMockFns, + hasMockCondition, + queueTableRows, + resetDbChainMock, + schemaMock, +} from '@sim/testing' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const { storage, prepareChat, executeChat, hardDelete, billing, decrement, reRoot } = vi.hoisted( + () => ({ + storage: vi.fn(), + prepareChat: vi.fn(), + executeChat: vi.fn(), + hardDelete: vi.fn(), + billing: vi.fn(), + decrement: vi.fn(), + reRoot: vi.fn(), + }) +) +vi.mock('@/background/cleanup-logs', () => ({ legacyLargeValuePredicate: vi.fn() })) +vi.mock('@/background/cleanup-soft-deletes', () => ({ + reRootActiveFolderChildrenUnguarded: reRoot, +})) +vi.mock('@/lib/uploads', () => ({ + isUsingCloudStorage: () => true, + StorageService: { deleteFiles: storage }, +})) +vi.mock('@/lib/cleanup/chat-cleanup', () => ({ prepareChatCleanup: prepareChat })) +vi.mock('@/lib/knowledge/documents/service', () => ({ hardDeleteDocuments: hardDelete })) +vi.mock('@/lib/billing/storage', () => ({ + resolveStorageBillingContext: billing, + decrementStorageUsageForBillingContextInTx: decrement, +})) + +import { BoundedCleanup, type CleanupTransaction } from '@/lib/cleanup/bounded' +import type { CleanupType } from '@/lib/cleanup/bounded-types' +import { runBoundedLogScope } from '@/background/cleanup-logs-bounded' +import { runBoundedSoftDeleteScope } from '@/background/cleanup-soft-deletes-bounded' + +const scope = { + plan: 'free' as const, + workspaceIds: ['ws-one'], + retentionHours: 720, + label: 'test', + runGlobalHousekeeping: true, +} +function control(type: CleanupType, dryRun = true, limit = 1) { + return new BoundedCleanup( + { limits: { [type]: limit }, batchSize: 1, dryRun, requestId: 'test' }, + async () => {}, + Date.now, + async (query) => query(dbChainMock.db as CleanupTransaction) + ) +} +beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + storage.mockResolvedValue({ deleted: 1, failed: [] }) + prepareChat.mockResolvedValue({ execute: executeChat }) +}) + +describe('requested cleanup stages', () => { + const targets = [ + ['workflowLogs', schemaMock.workflowExecutionLogs], + ['jobLogs', schemaMock.jobExecutionLogs], + ['largeValues', schemaMock.executionLargeValues], + ['legacyLargeValues', schemaMock.workspaceFiles], + ['orphanSnapshots', schemaMock.workflowExecutionSnapshots], + ['workflows', schemaMock.workflow], + ['chats', schemaMock.copilotChats], + ['legacyFiles', schemaMock.workspaceFile], + ['files', schemaMock.workspaceFiles], + ['knowledgeBases', schemaMock.knowledgeBase], + ['folders', schemaMock.folder], + ['userTables', schemaMock.userTableDefinitions], + ['memories', schemaMock.memory], + ['mcpServers', schemaMock.mcpServers], + ['workflowMcpServers', schemaMock.workflowMcpServer], + ['orphanKnowledgeBaseBindings', schemaMock.workspaceFiles], + ] as const + it.each(targets)( + 'dry run of %s selects only that stage and has no side effects', + async (type, table) => { + queueTableRows(table, [{ id: 'root-one', key: 'key-one', files: [{ key: 'attached-file' }] }]) + const run = control(type) + const runner = + targets.findIndex(([candidate]) => candidate === type) < 5 + ? runBoundedLogScope + : runBoundedSoftDeleteScope + await runner(scope, run) + expect(run.progress.stages[type]?.selected).toBe(1) + expect(Object.keys(run.progress.stages)).toEqual([type]) + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + expect(dbChainMockFns.update).not.toHaveBeenCalled() + for (const effect of [storage, prepareChat, hardDelete, billing, decrement, reRoot]) + expect(effect).not.toHaveBeenCalled() + } + ) + it.each(['staleReferences', 'staleDependencies', 'largeValueTombstones'] as const)( + 'dry run of %s uses SELECT only', + async (type) => { + dbChainMockFns.execute.mockResolvedValueOnce([{ id: 'metadata-one' }]) + const run = control(type) + await runBoundedLogScope(scope, run) + expect(run.progress.stages[type]?.selected).toBe(1) + expect(dbChainMockFns.execute).toHaveBeenCalledTimes(1) + expect(dbChainMockFns.execute.mock.calls[0][0].strings.join('')).toMatch(/^SELECT /) + } + ) + it('keeps the same log budget across workspace chunks', async () => { + const ids = Array.from({ length: 51 }, (_, index) => `ws-${index}`) + queueTableRows(schemaMock.jobExecutionLogs, [{ id: 'one' }]) + queueTableRows(schemaMock.jobExecutionLogs, []) + queueTableRows(schemaMock.jobExecutionLogs, [{ id: 'two' }]) + dbChainMockFns.returning.mockResolvedValue([{ id: 'deleted' }]) + const run = control('jobLogs', false, 2) + await runBoundedLogScope({ ...scope, workspaceIds: ids }, run) + expect(run.progress.stages.jobLogs).toMatchObject({ selected: 2, deleted: 2 }) + expect(dbChainMockFns.delete).toHaveBeenCalledTimes(2) + }) + it('stops before root deletion if attached log storage fails', async () => { + queueTableRows(schemaMock.workflowExecutionLogs, [{ id: 'one', files: [{ key: 'blob' }] }]) + storage.mockResolvedValue({ deleted: 0, failed: [{ key: 'blob', error: 'unavailable' }] }) + const run = control('workflowLogs', false) + await expect(runBoundedLogScope(scope, run)).rejects.toThrow('storage deletions failed') + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + expect(run.progress.stages.workflowLogs).toMatchObject({ + selected: 1, + deleted: 0, + filesFailed: 1, + }) + }) + it('keeps workflow chat side effects even with a zero chat budget', async () => { + queueTableRows(schemaMock.workflow, [{ id: 'workflow-one' }]) + queueTableRows(schemaMock.copilotChats, [{ id: 'child-chat' }]) + queueTableRows(schemaMock.copilotChats, []) + dbChainMockFns.returning.mockResolvedValue([{ id: 'workflow-one' }]) + const run = control('workflows', false) + await runBoundedSoftDeleteScope(scope, run) + expect(prepareChat).toHaveBeenCalledWith( + ['child-chat'], + 'test', + expect.objectContaining({ type: 'workflows' }) + ) + expect(executeChat).toHaveBeenCalledTimes(1) + expect(run.progress.stages.workflows?.deleted).toBe(1) + expect(run.progress.stages.chats).toBeUndefined() + }) + it('counts committed workflow deletion when backend cleanup subsequently fails', async () => { + queueTableRows(schemaMock.workflow, [{ id: 'workflow-one' }]) + dbChainMockFns.returning.mockResolvedValue([{ id: 'workflow-one' }]) + executeChat.mockRejectedValueOnce(new Error('backend failure')) + const run = control('workflows', false) + await expect(runBoundedSoftDeleteScope(scope, run)).rejects.toThrow('backend failure') + expect(run.progress.stages.workflows?.deleted).toBe(1) + }) + it('applies the shared file budget across workspace and organization scopes', async () => { + const run = control('files', true, 2) + queueTableRows(schemaMock.workspaceFiles, [{ id: 'workspace-file' }]) + queueTableRows(schemaMock.workspaceFiles, []) + await runBoundedSoftDeleteScope(scope, run) + queueTableRows(schemaMock.workspaceFiles, [{ id: 'organization-file' }]) + await runBoundedSoftDeleteScope( + { ...scope, workspaceIds: [], organizationIds: ['org-one'] }, + run + ) + expect(run.progress.stages.files?.selected).toBe(2) + expect(storage).not.toHaveBeenCalled() + }) +}) + +describe('bounded file billing', () => { + it('rechecks the exact payer workspace and decrements only bytes actually deleted', async () => { + queueTableRows(schemaMock.workspaceFiles, [ + { id: 'file-one', key: 'blob', context: 'workspace', workspaceId: 'ws-one', sizeBytes: 100 }, + ]) + billing.mockResolvedValue({ workspaceId: 'ws-one' }) + dbChainMockFns.returning.mockResolvedValue([{ id: 'file-one', sizeBytes: 40 }]) + const run = control('files', false) + await runBoundedSoftDeleteScope({ ...scope, workspaceIds: ['ws-one', 'ws-two'] }, run) + expect(decrement).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ workspaceId: 'ws-one' }), + 40 + ) + expect( + dbChainMockFns.where.mock.calls.some(([predicate]) => + hasMockCondition( + predicate, + (condition) => + condition.type === 'eq' && + condition.left === schemaMock.workspaceFiles.workspaceId && + condition.right === 'ws-one' + ) + ) + ).toBe(true) + expect(run.progress.stages.files?.deleted).toBe(1) + }) + it('validates canonical sizes before deleting any storage', async () => { + queueTableRows(schemaMock.workspaceFiles, [ + { id: 'file-one', key: 'blob', context: 'workspace', workspaceId: 'ws-one', sizeBytes: null }, + ]) + await expect(runBoundedSoftDeleteScope(scope, control('files', false))).rejects.toThrow( + 'canonical size_bytes' + ) + expect(storage).not.toHaveBeenCalled() + expect(dbChainMockFns.delete).not.toHaveBeenCalled() + }) +}) diff --git a/apps/sim/background/cleanup-logs-bounded.ts b/apps/sim/background/cleanup-logs-bounded.ts new file mode 100644 index 00000000000..6a3bc9264bc --- /dev/null +++ b/apps/sim/background/cleanup-logs-bounded.ts @@ -0,0 +1,157 @@ +import { + executionLargeValues, + jobExecutionLogs, + pausedExecutions, + workflowExecutionLogs, + workflowExecutionSnapshots, + workspaceFiles, +} from '@sim/db/schema' +import { chunkArray } from '@sim/utils/helpers' +import { and, asc, eq, inArray, isNull, lt, notInArray, sql } from 'drizzle-orm' +import type { CleanupJobPayload } from '@/lib/billing/cleanup-dispatcher' +import type { BoundedCleanup } from '@/lib/cleanup/bounded' +import { boundedDelete } from '@/lib/cleanup/bounded-delete' +import { pruneBoundedLargeValueMetadata } from '@/lib/cleanup/bounded-large-value-metadata' +import { deleteBoundedStorage, tombstoneBoundedFiles } from '@/lib/cleanup/bounded-storage' +import { + LIVE_PAUSED_REFERENCE_STATUSES, + unreferencedLargeValuePredicate, +} from '@/lib/execution/payloads/large-value-metadata' +import { isUsingCloudStorage } from '@/lib/uploads' +import { legacyLargeValuePredicate } from '@/background/cleanup-logs' + +/** Retention guards and mandatory storage effects match the scheduled log cleanup. */ +export async function runBoundedLogScope(payload: CleanupJobPayload, control: BoundedCleanup) { + const cutoff = new Date(Date.now() - payload.retentionHours * 3600_000) + for (const ids of chunkArray(payload.workspaceIds, 50)) { + const logsEligible = and( + inArray(workflowExecutionLogs.workspaceId, ids), + lt(workflowExecutionLogs.startedAt, cutoff), + sql`NOT EXISTS (SELECT 1 FROM ${pausedExecutions} pe WHERE pe.execution_id = ${workflowExecutionLogs.executionId} AND pe.status IN ${LIVE_PAUSED_REFERENCE_STATUSES})` + ) + await control.batches( + 'workflowLogs', + (limit, seen) => + control.query(async (tx) => + tx + .select({ id: workflowExecutionLogs.id, files: workflowExecutionLogs.files }) + .from(workflowExecutionLogs) + .where( + and( + logsEligible, + seen.length ? notInArray(workflowExecutionLogs.id, seen) : undefined + ) + ) + .orderBy( + asc(workflowExecutionLogs.workspaceId), + asc(workflowExecutionLogs.startedAt), + asc(workflowExecutionLogs.id) + ) + .limit(limit) + ), + (row) => row.id, + async (rows) => { + for (const row of rows) { + const keys = Array.isArray(row.files) + ? row.files.flatMap((file) => + file && typeof file === 'object' && 'key' in file && typeof file.key === 'string' + ? [file.key] + : [] + ) + : [] + await deleteBoundedStorage(control, 'workflowLogs', keys, 'execution') + if (isUsingCloudStorage()) await tombstoneBoundedFiles(control, keys) + } + const deleted = await control.query(async (tx) => + tx + .delete(workflowExecutionLogs) + .where( + and( + logsEligible, + inArray( + workflowExecutionLogs.id, + rows.map((row) => row.id) + ) + ) + ) + .returning({ id: workflowExecutionLogs.id }) + ) + await control.deleted('workflowLogs', deleted.length) + } + ) + await boundedDelete( + control, + 'jobLogs', + jobExecutionLogs, + jobExecutionLogs.id, + and(inArray(jobExecutionLogs.workspaceId, ids), lt(jobExecutionLogs.startedAt, cutoff)) + ) + + for (const legacy of [false, true]) { + const type = legacy ? 'legacyLargeValues' : 'largeValues' + const table = legacy ? workspaceFiles : executionLargeValues + const eligible = legacy + ? and( + inArray(workspaceFiles.workspaceId, ids), + eq(workspaceFiles.context, 'execution'), + isNull(workspaceFiles.deletedAt), + lt(workspaceFiles.uploadedAt, new Date(cutoff.getTime() - 30 * 86400_000)), + legacyLargeValuePredicate() + ) + : and( + inArray(executionLargeValues.workspaceId, ids), + isNull(executionLargeValues.deletedAt), + lt(executionLargeValues.createdAt, new Date(cutoff.getTime() - 7 * 86400_000)), + unreferencedLargeValuePredicate() + ) + await control.batches( + type, + (limit, seen) => + control.query(async (tx) => + tx + .select({ key: table.key }) + .from(table) + .where(and(eligible, seen.length ? notInArray(table.key, seen) : undefined)) + .orderBy( + asc(table.workspaceId), + asc(legacy ? workspaceFiles.uploadedAt : executionLargeValues.createdAt), + asc(table.key) + ) + .limit(limit) + ), + (row) => row.key, + async (rows) => { + if (!isUsingCloudStorage()) return + const keys = rows.map((row) => row.key) + await deleteBoundedStorage(control, type, keys, 'execution') + if (!legacy) + await control.query(async (tx) => { + await tx + .update(executionLargeValues) + .set({ deletedAt: new Date() }) + .where(inArray(executionLargeValues.key, keys)) + }) + await control.deleted(type, keys.length) + await tombstoneBoundedFiles(control, keys) + } + ) + } + + await pruneBoundedLargeValueMetadata(control, ids) + if (control.stopped()) return + } + if (payload.runGlobalHousekeeping && payload.plan === 'free') { + const cutoff = new Date() + cutoff.setDate(cutoff.getDate() - (Math.floor(payload.retentionHours / 24) + 1)) + await boundedDelete( + control, + 'orphanSnapshots', + workflowExecutionSnapshots, + workflowExecutionSnapshots.id, + and( + lt(workflowExecutionSnapshots.createdAt, cutoff), + sql`NOT EXISTS (SELECT 1 FROM ${workflowExecutionLogs} wel WHERE wel.state_snapshot_id = ${workflowExecutionSnapshots.id})` + ) + ) + } +} diff --git a/apps/sim/background/cleanup-logs.test.ts b/apps/sim/background/cleanup-logs.test.ts index 2b1adb3cdc5..5035feb5a95 100644 --- a/apps/sim/background/cleanup-logs.test.ts +++ b/apps/sim/background/cleanup-logs.test.ts @@ -199,7 +199,7 @@ describe('cleanup logs worker', () => { it('caps Trigger.dev concurrency for log cleanup tasks', () => { expect(cleanupLogsTask).toMatchObject({ - queue: { concurrencyLimit: 2 }, + queue: { name: 'retention-cleanup', concurrencyLimit: 1 }, }) }) }) diff --git a/apps/sim/background/cleanup-logs.ts b/apps/sim/background/cleanup-logs.ts index 2d560ef74c0..729be4641d5 100644 --- a/apps/sim/background/cleanup-logs.ts +++ b/apps/sim/background/cleanup-logs.ts @@ -18,6 +18,8 @@ import { chunkedBatchDelete, type TableCleanupResult, } from '@/lib/cleanup/batch-delete' +import type { BoundedCleanupPayload } from '@/lib/cleanup/bounded-types' +import { retentionCleanupQueue } from '@/lib/cleanup/queue' import { LIVE_PAUSED_REFERENCE_STATUSES, markLargeValuesDeleted, @@ -43,7 +45,6 @@ const WORKFLOW_LOG_CLEANUP_BATCH_SIZE = 500 const WORKFLOW_LOG_CLEANUP_MAX_BATCHES = 50 const WORKFLOW_LOG_CLEANUP_ROW_LIMIT = WORKFLOW_LOG_CLEANUP_BATCH_SIZE * WORKFLOW_LOG_CLEANUP_MAX_BATCHES -const LOG_CLEANUP_CONCURRENCY_LIMIT = 2 const LARGE_VALUE_CLEANUP_BATCH_SIZE = 500 const LARGE_VALUE_CLEANUP_TOTAL_KEY_LIMIT = 5_000 const LARGE_VALUE_CLEANUP_GRACE_HOURS = 7 * 24 @@ -231,93 +232,7 @@ async function cleanupLegacyLargeExecutionValues( eq(workspaceFiles.context, 'execution'), isNull(workspaceFiles.deletedAt), lt(workspaceFiles.uploadedAt, legacyRetentionDate), - sql`${workspaceFiles.key} LIKE 'execution/%/%/%/large-value-lv_%.json'`, - sql`NOT EXISTS ( - SELECT 1 - FROM ${executionLargeValues} AS registered_value - WHERE registered_value.key = ${workspaceFiles.key} - )`, - sql`NOT EXISTS ( - SELECT 1 - FROM ${executionLargeValueReferences} AS ref - WHERE ref.key = ${workspaceFiles.key} - AND ( - ( - ref.source = 'execution_log' - AND EXISTS ( - SELECT 1 - FROM ${workflowExecutionLogs} AS ref_wel - WHERE ref_wel.execution_id = ref.execution_id - ) - ) - OR ( - ref.source = 'paused_snapshot' - AND EXISTS ( - SELECT 1 - FROM ${pausedExecutions} AS ref_pe - WHERE ref_pe.execution_id = ref.execution_id - AND ref_pe.status IN ${LIVE_PAUSED_REFERENCE_STATUSES} - ) - ) - ) - )`, - sql`NOT EXISTS ( - SELECT 1 - FROM ${executionLargeValueDependencies} AS dependency - INNER JOIN ${executionLargeValues} AS parent_value - ON parent_value.key = dependency.parent_key - AND parent_value.deleted_at IS NULL - WHERE dependency.child_key = ${workspaceFiles.key} - AND dependency.workspace_id = ${workspaceFiles.workspaceId} - AND ( - EXISTS ( - SELECT 1 - FROM ${workflowExecutionLogs} AS parent_owner_wel - WHERE parent_owner_wel.execution_id = parent_value.owner_execution_id - ) - OR EXISTS ( - SELECT 1 - FROM ${pausedExecutions} AS parent_owner_pe - WHERE parent_owner_pe.execution_id = parent_value.owner_execution_id - AND parent_owner_pe.status IN ${LIVE_PAUSED_REFERENCE_STATUSES} - ) - OR EXISTS ( - SELECT 1 - FROM ${executionLargeValueReferences} AS parent_ref - WHERE parent_ref.key = parent_value.key - AND ( - ( - parent_ref.source = 'execution_log' - AND EXISTS ( - SELECT 1 - FROM ${workflowExecutionLogs} AS parent_ref_wel - WHERE parent_ref_wel.execution_id = parent_ref.execution_id - ) - ) - OR ( - parent_ref.source = 'paused_snapshot' - AND EXISTS ( - SELECT 1 - FROM ${pausedExecutions} AS parent_ref_pe - WHERE parent_ref_pe.execution_id = parent_ref.execution_id - AND parent_ref_pe.status IN ${LIVE_PAUSED_REFERENCE_STATUSES} - ) - ) - ) - ) - ) - )`, - sql`NOT EXISTS ( - SELECT 1 - FROM ${workflowExecutionLogs} AS owner_wel - WHERE owner_wel.execution_id = split_part(${workspaceFiles.key}, '/', 4) - )`, - sql`NOT EXISTS ( - SELECT 1 - FROM ${pausedExecutions} AS pe - WHERE pe.execution_id = split_part(${workspaceFiles.key}, '/', 4) - AND pe.status IN ${LIVE_PAUSED_REFERENCE_STATUSES} - )` + legacyLargeValuePredicate() ) ) .orderBy( @@ -487,6 +402,106 @@ export async function runCleanupLogs(payload: CleanupJobPayload): Promise export const cleanupLogsTask = task({ id: 'cleanup-logs', machine: 'large-1x', - queue: { concurrencyLimit: LOG_CLEANUP_CONCURRENCY_LIMIT }, - run: runCleanupLogs, + queue: retentionCleanupQueue, + run: async (payload: CleanupJobPayload | BoundedCleanupPayload) => { + if ('mode' in payload && payload.mode === 'bounded') { + const { runBoundedCleanup } = await import('@/lib/cleanup/bounded-runner') + const { runBoundedLogScope } = await import('@/background/cleanup-logs-bounded') + return runBoundedCleanup('cleanup-logs', payload, runBoundedLogScope) + } + return runCleanupLogs(payload as CleanupJobPayload) + }, }) + +/** Shared reference guards for legacy large values; bounded and scheduled cleanup must agree. */ +export function legacyLargeValuePredicate() { + return and( + sql`${workspaceFiles.key} LIKE 'execution/%/%/%/large-value-lv_%.json'`, + sql`NOT EXISTS ( + SELECT 1 + FROM ${executionLargeValues} AS registered_value + WHERE registered_value.key = ${workspaceFiles.key} + )`, + sql`NOT EXISTS ( + SELECT 1 + FROM ${executionLargeValueReferences} AS ref + WHERE ref.key = ${workspaceFiles.key} + AND ( + ( + ref.source = 'execution_log' + AND EXISTS ( + SELECT 1 + FROM ${workflowExecutionLogs} AS ref_wel + WHERE ref_wel.execution_id = ref.execution_id + ) + ) + OR ( + ref.source = 'paused_snapshot' + AND EXISTS ( + SELECT 1 + FROM ${pausedExecutions} AS ref_pe + WHERE ref_pe.execution_id = ref.execution_id + AND ref_pe.status IN ${LIVE_PAUSED_REFERENCE_STATUSES} + ) + ) + ) + )`, + sql`NOT EXISTS ( + SELECT 1 + FROM ${executionLargeValueDependencies} AS dependency + INNER JOIN ${executionLargeValues} AS parent_value + ON parent_value.key = dependency.parent_key + AND parent_value.deleted_at IS NULL + WHERE dependency.child_key = ${workspaceFiles.key} + AND dependency.workspace_id = ${workspaceFiles.workspaceId} + AND ( + EXISTS ( + SELECT 1 + FROM ${workflowExecutionLogs} AS parent_owner_wel + WHERE parent_owner_wel.execution_id = parent_value.owner_execution_id + ) + OR EXISTS ( + SELECT 1 + FROM ${pausedExecutions} AS parent_owner_pe + WHERE parent_owner_pe.execution_id = parent_value.owner_execution_id + AND parent_owner_pe.status IN ${LIVE_PAUSED_REFERENCE_STATUSES} + ) + OR EXISTS ( + SELECT 1 + FROM ${executionLargeValueReferences} AS parent_ref + WHERE parent_ref.key = parent_value.key + AND ( + ( + parent_ref.source = 'execution_log' + AND EXISTS ( + SELECT 1 + FROM ${workflowExecutionLogs} AS parent_ref_wel + WHERE parent_ref_wel.execution_id = parent_ref.execution_id + ) + ) + OR ( + parent_ref.source = 'paused_snapshot' + AND EXISTS ( + SELECT 1 + FROM ${pausedExecutions} AS parent_ref_pe + WHERE parent_ref_pe.execution_id = parent_ref.execution_id + AND parent_ref_pe.status IN ${LIVE_PAUSED_REFERENCE_STATUSES} + ) + ) + ) + ) + ) + )`, + sql`NOT EXISTS ( + SELECT 1 + FROM ${workflowExecutionLogs} AS owner_wel + WHERE owner_wel.execution_id = split_part(${workspaceFiles.key}, '/', 4) + )`, + sql`NOT EXISTS ( + SELECT 1 + FROM ${pausedExecutions} AS pe + WHERE pe.execution_id = split_part(${workspaceFiles.key}, '/', 4) + AND pe.status IN ${LIVE_PAUSED_REFERENCE_STATUSES} + )` + ) +} diff --git a/apps/sim/background/cleanup-soft-deletes-bounded.ts b/apps/sim/background/cleanup-soft-deletes-bounded.ts new file mode 100644 index 00000000000..4196a9dbac7 --- /dev/null +++ b/apps/sim/background/cleanup-soft-deletes-bounded.ts @@ -0,0 +1,352 @@ +import { db } from '@sim/db' +import { + copilotChats, + document, + folder, + knowledgeBase, + mcpServers, + memory, + userTableDefinitions, + workflow, + workflowMcpServer, + workspaceFile, + workspaceFiles, +} from '@sim/db/schema' +import { chunkArray } from '@sim/utils/helpers' +import { and, asc, eq, gt, inArray, isNotNull, isNull, lt, notInArray, sql } from 'drizzle-orm' +import type { CleanupJobPayload } from '@/lib/billing/cleanup-dispatcher' +import { + decrementStorageUsageForBillingContextInTx, + resolveStorageBillingContext, +} from '@/lib/billing/storage' +import { type BoundedCleanup, setCleanupTimeouts } from '@/lib/cleanup/bounded' +import { boundedDelete } from '@/lib/cleanup/bounded-delete' +import { deleteBoundedStorage } from '@/lib/cleanup/bounded-storage' +import type { CleanupType } from '@/lib/cleanup/bounded-types' +import { prepareChatCleanup } from '@/lib/cleanup/chat-cleanup' +import { + type CleanupOwnerScope, + cleanupOwnerCondition, + resolveCleanupOwnerScope, +} from '@/lib/cleanup/resource-scope' +import { hardDeleteDocuments } from '@/lib/knowledge/documents/service' +import type { StorageContext } from '@/lib/uploads' +import { getWorkspaceFileSize } from '@/lib/uploads/shared/types' +import { reRootActiveFolderChildrenUnguarded } from '@/background/cleanup-soft-deletes' + +/** Budgets count parent selections; required cascading effects are kept with their parent batch. */ +export async function runBoundedSoftDeleteScope( + payload: CleanupJobPayload, + control: BoundedCleanup +) { + const cutoff = new Date(Date.now() - payload.retentionHours * 3600_000) + const owner = resolveCleanupOwnerScope(payload) + for (const ids of chunkArray(owner.ids, 50)) { + const scope = { ...owner, ids } + let chatCleanup: Awaited> | undefined + if (scope.kind === 'workspace') { + await boundedDelete( + control, + 'workflows', + workflow, + workflow.id, + and( + inArray(workflow.workspaceId, ids), + isNotNull(workflow.archivedAt), + lt(workflow.archivedAt, cutoff) + ), + { + before: async (workflowIds) => { + // Collect every child chat; a parent limit cannot silently truncate its required cleanup. + const chatIds: string[] = [] + let after: string | undefined + while (true) { + control.assertTimeRemaining() + const rows = await control.query(async (tx) => + tx + .select({ id: copilotChats.id }) + .from(copilotChats) + .where( + and( + inArray(copilotChats.workflowId, workflowIds), + after ? gt(copilotChats.id, after) : undefined + ) + ) + .orderBy(asc(copilotChats.id)) + .limit(control.options.batchSize) + ) + if (rows.length === 0) break + chatIds.push(...rows.map((row) => row.id)) + after = rows[rows.length - 1].id + } + chatCleanup = await prepareChatCleanup(chatIds, payload.label, { + control, + type: 'workflows', + }) + }, + after: async () => { + await chatCleanup?.execute() + }, + } + ) + } + await boundedDelete( + control, + 'chats', + copilotChats, + copilotChats.id, + and( + cleanupOwnerCondition(copilotChats, scope), + isNotNull(copilotChats.deletedAt), + lt(copilotChats.deletedAt, cutoff) + ), + { + before: async (chatIds) => { + chatCleanup = await prepareChatCleanup(chatIds, payload.label, { control, type: 'chats' }) + }, + after: async () => { + await chatCleanup?.execute() + }, + } + ) + await cleanupFiles(control, scope, cutoff) + await boundedDelete( + control, + 'knowledgeBases', + knowledgeBase, + knowledgeBase.id, + and( + cleanupOwnerCondition(knowledgeBase, scope), + isNotNull(knowledgeBase.deletedAt), + lt(knowledgeBase.deletedAt, cutoff) + ), + { + before: async (kbIds) => { + // Existing ledger/outbox implementation owns document and embedding deletion. + while (true) { + control.assertTimeRemaining() + const rows = await control.query(async (tx) => + tx + .select({ id: document.id }) + .from(document) + .where(inArray(document.knowledgeBaseId, kbIds)) + .orderBy(asc(document.id)) + .limit(control.options.batchSize) + ) + if (rows.length === 0) break + const deleted = await hardDeleteDocuments( + rows.map((row) => row.id), + payload.label, + undefined, + undefined, + undefined, + undefined, + control.query + ) + if (deleted !== rows.length) + throw new Error('Knowledge-base document cleanup did not delete its selected batch') + } + }, + } + ) + if (scope.kind === 'workspace') { + const targets = [ + { + type: 'folders', + table: folder, + date: folder.deletedAt, + extra: inArray(folder.resourceType, ['workflow', 'file', 'knowledge_base', 'table']), + }, + { type: 'userTables', table: userTableDefinitions, date: userTableDefinitions.archivedAt }, + { type: 'memories', table: memory, date: memory.deletedAt }, + { type: 'mcpServers', table: mcpServers, date: mcpServers.deletedAt }, + { type: 'workflowMcpServers', table: workflowMcpServer, date: workflowMcpServer.deletedAt }, + ] as const + for (const target of targets) { + await boundedDelete( + control, + target.type, + target.table, + target.table.id, + and( + inArray(target.table.workspaceId, ids), + isNotNull(target.date), + lt(target.date, cutoff), + 'extra' in target ? target.extra : undefined + ), + target.type === 'folders' + ? { + before: (folderIds) => + control.query((tx) => + reRootActiveFolderChildrenUnguarded(folderIds, cutoff, payload.label, tx, true) + ), + } + : {} + ) + } + } + await cleanupOrphanBindings(control, scope) + if (control.stopped()) return + } +} + +async function cleanupFiles(control: BoundedCleanup, scope: CleanupOwnerScope, cutoff: Date) { + if (scope.kind === 'workspace') { + const eligible = and( + inArray(workspaceFile.workspaceId, scope.ids), + isNotNull(workspaceFile.deletedAt), + lt(workspaceFile.deletedAt, cutoff) + ) + await control.batches( + 'legacyFiles', + (limit, seen) => + control.query(async (tx) => + tx + .select({ id: workspaceFile.id, key: workspaceFile.key }) + .from(workspaceFile) + .where(and(eligible, seen.length ? notInArray(workspaceFile.id, seen) : undefined)) + .orderBy(asc(workspaceFile.id)) + .limit(limit) + ), + (row) => row.id, + async (rows) => { + await deleteBoundedStorage( + control, + 'legacyFiles', + rows.map((row) => row.key), + 'workspace' + ) + const deleted = await control.query(async (tx) => + tx + .delete(workspaceFile) + .where( + and( + eligible, + inArray( + workspaceFile.id, + rows.map((row) => row.id) + ) + ) + ) + .returning({ id: workspaceFile.id }) + ) + await control.deleted('legacyFiles', deleted.length) + } + ) + } + const eligible = and( + cleanupOwnerCondition(workspaceFiles, scope), + isNotNull(workspaceFiles.deletedAt), + lt(workspaceFiles.deletedAt, cutoff), + scope.kind === 'organization' ? eq(workspaceFiles.context, 'knowledge-base') : undefined + ) + await control.batches( + 'files', + (limit, seen) => + control.query(async (tx) => + tx + .select({ + id: workspaceFiles.id, + key: workspaceFiles.key, + context: workspaceFiles.context, + workspaceId: workspaceFiles.workspaceId, + sizeBytes: workspaceFiles.sizeBytes, + }) + .from(workspaceFiles) + .where(and(eligible, seen.length ? notInArray(workspaceFiles.id, seen) : undefined)) + .orderBy(asc(workspaceFiles.id)) + .limit(limit) + ), + (row) => row.id, + async (rows) => { + for (const row of rows) getWorkspaceFileSize(row) + // Delete exact selected versions and couple billable deletions with their ledger decrement. + // One row at a time keeps different payer/context transactions independent and observable. + for (const row of rows) { + const billing = + row.context === 'workspace' + ? await control.query((executor) => { + if (!row.workspaceId) throw new Error(`Billable file ${row.id} has no workspace`) + return resolveStorageBillingContext(row.workspaceId, { executor }) + }) + : undefined + await deleteBoundedStorage(control, 'files', [row.key], row.context as StorageContext) + const remove = async (tx: Parameters[0]>[0]) => { + const deleted = await tx + .delete(workspaceFiles) + .where( + and( + eligible, + eq(workspaceFiles.id, row.id), + eq(workspaceFiles.context, row.context), + billing ? eq(workspaceFiles.workspaceId, billing.workspaceId) : undefined + ) + ) + .returning({ id: workspaceFiles.id, sizeBytes: workspaceFiles.sizeBytes }) + if (billing) + await decrementStorageUsageForBillingContextInTx( + tx, + billing, + deleted.reduce((sum, file) => sum + getWorkspaceFileSize(file), 0) + ) + return deleted.length + } + const deleted = billing + ? await db.transaction(async (tx) => { + await setCleanupTimeouts(tx) + return remove(tx) + }) + : await control.query(remove) + await control.deleted('files', deleted) + } + } + ) +} + +async function cleanupOrphanBindings(control: BoundedCleanup, scope: CleanupOwnerScope) { + const type: CleanupType = 'orphanKnowledgeBaseBindings' + const eligible = and( + cleanupOwnerCondition(workspaceFiles, scope), + eq(workspaceFiles.context, 'knowledge-base'), + isNull(workspaceFiles.deletedAt), + lt(workspaceFiles.uploadedAt, new Date(Date.now() - 7 * 86400_000)), + sql`NOT EXISTS (SELECT 1 FROM ${document} doc WHERE doc.storage_key = ${workspaceFiles.key})` + ) + await control.batches( + type, + (limit, seen) => + control.query(async (tx) => + tx + .select({ id: workspaceFiles.id, key: workspaceFiles.key }) + .from(workspaceFiles) + .where(and(eligible, seen.length ? notInArray(workspaceFiles.id, seen) : undefined)) + .orderBy(asc(workspaceFiles.id)) + .limit(limit) + ), + (row) => row.id, + async (rows) => { + await deleteBoundedStorage( + control, + type, + rows.map((row) => row.key), + 'knowledge-base' + ) + const deleted = await control.query(async (tx) => + tx + .update(workspaceFiles) + .set({ deletedAt: new Date() }) + .where( + and( + eligible, + inArray( + workspaceFiles.id, + rows.map((row) => row.id) + ) + ) + ) + .returning({ id: workspaceFiles.id }) + ) + await control.deleted(type, deleted.length) + } + ) +} diff --git a/apps/sim/background/cleanup-soft-deletes.test.ts b/apps/sim/background/cleanup-soft-deletes.test.ts index 2d6816d525a..6f08bd697fe 100644 --- a/apps/sim/background/cleanup-soft-deletes.test.ts +++ b/apps/sim/background/cleanup-soft-deletes.test.ts @@ -429,7 +429,12 @@ describe('folder cleanup target', () => { await onBatch([{ id: 'folder-1' }]) - expect(mockAllocateUniqueWorkspaceFileName).toHaveBeenCalledWith('ws-1', 'report.pdf', null) + expect(mockAllocateUniqueWorkspaceFileName).toHaveBeenCalledWith( + 'ws-1', + 'report.pdf', + null, + undefined + ) expect(dbChainMockFns.set).toHaveBeenCalledWith({ folderId: null, originalName: 'report (2).pdf', diff --git a/apps/sim/background/cleanup-soft-deletes.ts b/apps/sim/background/cleanup-soft-deletes.ts index 2f48f1d51d7..0de51000e60 100644 --- a/apps/sim/background/cleanup-soft-deletes.ts +++ b/apps/sim/background/cleanup-soft-deletes.ts @@ -29,7 +29,9 @@ import { DEFAULT_DELETE_CHUNK_SIZE, selectRowsByIdChunks, } from '@/lib/cleanup/batch-delete' +import type { BoundedCleanupPayload } from '@/lib/cleanup/bounded-types' import { prepareChatCleanup } from '@/lib/cleanup/chat-cleanup' +import { retentionCleanupQueue } from '@/lib/cleanup/queue' import { type CleanupOwnerScope, cleanupOwnerCondition, @@ -467,12 +469,14 @@ async function reRootOne( preferred: () => Promise, withUniqueName: () => Promise, subject: string, - label: string + label: string, + strict = false ): Promise { try { await preferred() return } catch (error) { + if (strict) throw error logger.warn(`[${label}] Re-rooting ${subject} under its deduplicated name failed; retrying`, { error, }) @@ -517,10 +521,12 @@ async function reRootActiveFolderChildren( } } -async function reRootActiveFolderChildrenUnguarded( +export async function reRootActiveFolderChildrenUnguarded( folderIds: string[], retentionDate: Date, - label: string + label: string, + executor: Pick = cleanupDb, + strict = false ): Promise { /** * Re-asserted here, not just on the DELETE. `deleteFilter` already skips a folder restored @@ -534,7 +540,7 @@ async function reRootActiveFolderChildrenUnguarded( * Closing it properly means holding a row lock across select → onBatch → delete, which * nothing in this sweep does today. */ - const stillExpired = await cleanupDb + const stillExpired = await executor .select({ id: folderTable.id }) .from(folderTable) .where( @@ -548,7 +554,7 @@ async function reRootActiveFolderChildrenUnguarded( const expiredIds = stillExpired.map(({ id }) => id) if (expiredIds.length === 0) return - const workflows = await cleanupDb + const workflows = await executor .select({ id: workflow.id, name: workflow.name, workspaceId: workflow.workspaceId }) .from(workflow) .where(and(inArray(workflow.folderId, expiredIds), isNull(workflow.archivedAt))) @@ -558,23 +564,21 @@ async function reRootActiveFolderChildrenUnguarded( if (!workspaceId) continue await reRootOne( async () => { - const name = await deduplicateWorkflowName(row.name, workspaceId, null, cleanupDb) - await cleanupDb - .update(workflow) - .set({ folderId: null, name }) - .where(eq(workflow.id, row.id)) + const name = await deduplicateWorkflowName(row.name, workspaceId, null, executor) + await executor.update(workflow).set({ folderId: null, name }).where(eq(workflow.id, row.id)) }, () => - cleanupDb + executor .update(workflow) .set({ folderId: null, name: `${row.name} (${row.id})` }) .where(eq(workflow.id, row.id)), `workflow ${row.id}`, - label + label, + strict ) } - const files = await cleanupDb + const files = await executor .select({ id: workspaceFiles.id, originalName: workspaceFiles.originalName, @@ -597,20 +601,39 @@ async function reRootActiveFolderChildrenUnguarded( const originalName = await allocateUniqueWorkspaceFileName( workspaceId, row.originalName, - null + null, + strict + ? async (workspaceId, name) => { + const [existing] = await executor + .select({ id: workspaceFiles.id }) + .from(workspaceFiles) + .where( + and( + eq(workspaceFiles.workspaceId, workspaceId), + eq(workspaceFiles.context, 'workspace'), + isNull(workspaceFiles.folderId), + isNull(workspaceFiles.deletedAt), + eq(workspaceFiles.originalName, name) + ) + ) + .limit(1) + return Boolean(existing) + } + : undefined ) - await cleanupDb + await executor .update(workspaceFiles) .set({ folderId: null, originalName }) .where(eq(workspaceFiles.id, row.id)) }, () => - cleanupDb + executor .update(workspaceFiles) .set({ folderId: null, originalName: `${row.originalName} (${row.id})` }) .where(eq(workspaceFiles.id, row.id)), `workspace file ${row.id}`, - label + label, + strict ) } @@ -621,7 +644,7 @@ async function reRootActiveFolderChildrenUnguarded( * namespace where its name may already be taken — the identical 23505 stall. Covering only * workflows and files would leave the class half-closed. */ - const childFolders = await cleanupDb + const childFolders = await executor .select({ id: folderTable.id, name: folderTable.name, @@ -635,24 +658,25 @@ async function reRootActiveFolderChildrenUnguarded( await reRootOne( async () => { const name = await deduplicateFolderName( - cleanupDb, + executor, row.workspaceId, null, row.name, row.resourceType ) - await cleanupDb + await executor .update(folderTable) .set({ parentId: null, name }) .where(eq(folderTable.id, row.id)) }, () => - cleanupDb + executor .update(folderTable) .set({ parentId: null, name: `${row.name} (${row.id})` }) .where(eq(folderTable.id, row.id)), `folder ${row.id}`, - label + label, + strict ) } @@ -975,6 +999,15 @@ export async function runCleanupSoftDeletes(payload: CleanupJobPayload): Promise export const cleanupSoftDeletesTask = task({ id: 'cleanup-soft-deletes', machine: 'large-1x', - queue: { concurrencyLimit: 5 }, - run: runCleanupSoftDeletes, + queue: retentionCleanupQueue, + run: async (payload: CleanupJobPayload | BoundedCleanupPayload) => { + if ('mode' in payload && payload.mode === 'bounded') { + const { runBoundedCleanup } = await import('@/lib/cleanup/bounded-runner') + const { runBoundedSoftDeleteScope } = await import( + '@/background/cleanup-soft-deletes-bounded' + ) + return runBoundedCleanup('cleanup-soft-deletes', payload, runBoundedSoftDeleteScope) + } + return runCleanupSoftDeletes(payload as CleanupJobPayload) + }, }) diff --git a/apps/sim/lib/api/contracts/cleanup.test.ts b/apps/sim/lib/api/contracts/cleanup.test.ts new file mode 100644 index 00000000000..3dead3f3e2f --- /dev/null +++ b/apps/sim/lib/api/contracts/cleanup.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from 'vitest' +import { logsCleanupQuerySchema, softDeletesCleanupQuerySchema } from '@/lib/api/contracts/cleanup' + +describe('bounded cleanup contract', () => { + it('keeps an empty query in scheduled mode', () => { + expect(logsCleanupQuerySchema.parse({})).toBeUndefined() + expect(softDeletesCleanupQuerySchema.parse({})).toBeUndefined() + }) + it('defaults omitted budgets to zero and batches to 25', () => { + expect(logsCleanupQuerySchema.parse({ workflowLogs: '5', requestId: 'wave_1' })).toMatchObject({ + limits: { workflowLogs: 5, jobLogs: 0, orphanSnapshots: 0 }, + batchSize: 25, + dryRun: false, + }) + }) + it.each([ + { workflowLogs: '-1', requestId: 'r' }, + { workflowLogs: '1.5', requestId: 'r' }, + { workflowLogs: '5001', requestId: 'r' }, + { workflowLogs: '1e2', requestId: 'r' }, + { workflowLogs: '', requestId: 'r' }, + { workflowLogs: '1' }, + { workflowLogs: '1', requestId: 'r', batchSize: '501' }, + { workflowLogs: '1', requestId: 'r', batchSize: '0' }, + { workflowLogs: '1', requestId: 'r', dryRun: 'yes' }, + { workflowLogs: '1', requestId: 'r', workflows: '1' }, + { workflowLogs: ['1', '2'], requestId: 'r' }, + { requestId: 'r', dryRun: 'true' }, + { workflowLogs: '0', requestId: 'r' }, + ])('rejects malformed or ineffective requests: %j', (query) => { + expect(logsCleanupQuerySchema.safeParse(query).success).toBe(false) + }) +}) diff --git a/apps/sim/lib/api/contracts/cleanup.ts b/apps/sim/lib/api/contracts/cleanup.ts new file mode 100644 index 00000000000..cbbbc8eab6c --- /dev/null +++ b/apps/sim/lib/api/contracts/cleanup.ts @@ -0,0 +1,103 @@ +import { z } from 'zod' +import { defineRouteContract } from '@/lib/api/contracts/types' +import { + type BoundedCleanupOptions, + type CleanupType, + LOG_CLEANUP_TYPES, + SOFT_DELETE_CLEANUP_TYPES, +} from '@/lib/cleanup/bounded-types' + +const integerQuery = (min: number, max: number) => + z.string().regex(/^\d+$/).transform(Number).pipe(z.number().int().min(min).max(max)) + +function cleanupQuerySchema(types: readonly CleanupType[]) { + const limits = Object.fromEntries(types.map((type) => [type, integerQuery(0, 5000).optional()])) + return z + .object({ + ...limits, + batchSize: integerQuery(1, 500).optional(), + dryRun: z + .enum(['true', 'false']) + .transform((value) => value === 'true') + .optional(), + requestId: z + .string() + .regex(/^[A-Za-z0-9_-]{1,128}$/) + .optional(), + }) + .strict() + .transform((query, ctx): BoundedCleanupOptions | undefined => { + if (Object.keys(query).length === 0) return undefined + const budgets: BoundedCleanupOptions['limits'] = {} + for (const type of types) + budgets[type] = (query as Partial>)[type] ?? 0 + if (!Object.values(budgets).some((limit) => limit > 0)) { + ctx.addIssue({ code: 'custom', message: 'At least one positive cleanup limit is required' }) + return z.NEVER + } + if (!query.requestId) { + ctx.addIssue({ + code: 'custom', + path: ['requestId'], + message: 'requestId is required for bounded cleanup', + }) + return z.NEVER + } + return { + limits: budgets, + batchSize: query.batchSize ?? 25, + dryRun: query.dryRun ?? false, + requestId: query.requestId, + } + }) +} +export const logsCleanupQuerySchema = cleanupQuerySchema(LOG_CLEANUP_TYPES) +export const softDeletesCleanupQuerySchema = cleanupQuerySchema(SOFT_DELETE_CLEANUP_TYPES) +const cleanupResponseSchema = z.union([ + z.object({ + triggered: z.literal(true), + jobIds: z.array(z.string()), + jobCount: z.number(), + chunkCount: z.number(), + workspaceCount: z.number(), + }), + z.object({ + triggered: z.literal(true), + mode: z.literal('bounded'), + runId: z.string(), + requestId: z.string(), + batchSize: z.number().int(), + dryRun: z.boolean(), + limits: z.partialRecord( + z.enum([...LOG_CLEANUP_TYPES, ...SOFT_DELETE_CLEANUP_TYPES]), + z.number().int().min(0).max(5000) + ), + }), +]) +export const logsCleanupContract = defineRouteContract({ + method: 'GET', + path: '/api/logs/cleanup', + query: logsCleanupQuerySchema, + response: { mode: 'json', schema: cleanupResponseSchema, status: [200, 202] }, +}) +export const softDeletesCleanupContract = defineRouteContract({ + method: 'GET', + path: '/api/cron/cleanup-soft-deletes', + query: softDeletesCleanupQuerySchema, + response: { mode: 'json', schema: cleanupResponseSchema, status: [200, 202] }, +}) + +/** Validate direct Trigger invocations as well as HTTP calls. */ +export function validateBoundedCleanupOptions( + options: BoundedCleanupOptions, + types: readonly CleanupType[] +) { + return cleanupQuerySchema(types).parse({ + ...Object.fromEntries( + Object.entries(options.limits).map(([key, value]) => [key, String(value)]) + ), + batchSize: String(options.batchSize), + dryRun: String(options.dryRun), + requestId: options.requestId, + })! +} diff --git a/apps/sim/lib/billing/cleanup-dispatcher.test.ts b/apps/sim/lib/billing/cleanup-dispatcher.test.ts index fbf6a593652..4245d05abb0 100644 --- a/apps/sim/lib/billing/cleanup-dispatcher.test.ts +++ b/apps/sim/lib/billing/cleanup-dispatcher.test.ts @@ -11,12 +11,27 @@ import { } from '@sim/testing' import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest' -const { mockIsTriggerAvailable, mockGetOrganizationSubscription, mockEnqueue } = vi.hoisted(() => ({ +const { + mockIsTriggerAvailable, + mockGetOrganizationSubscription, + mockEnqueue, + mockTrigger, + mockRetrieve, + mockBatchTrigger, +} = vi.hoisted(() => ({ + mockTrigger: vi.fn(), + mockRetrieve: vi.fn(), + mockBatchTrigger: vi.fn(), mockIsTriggerAvailable: vi.fn(), mockGetOrganizationSubscription: vi.fn(), mockEnqueue: vi.fn(), })) +vi.mock('@trigger.dev/sdk', () => ({ + tasks: { trigger: mockTrigger, batchTrigger: mockBatchTrigger }, + runs: { retrieve: mockRetrieve }, +})) + vi.mock('@/lib/billing/core/billing', () => ({ getOrganizationSubscription: mockGetOrganizationSubscription, })) @@ -36,7 +51,11 @@ vi.mock('@/lib/workspaces/policy', () => ({ isOrganizationWorkspace: vi.fn(), })) -import { dispatchCleanupJobs } from '@/lib/billing/cleanup-dispatcher' +import { + dispatchBoundedCleanup, + dispatchCleanupJobs, + forEachCleanupChunk, +} from '@/lib/billing/cleanup-dispatcher' afterAll(resetEnvFlagsMock) @@ -222,3 +241,102 @@ describe('organization-owned Search retention dispatch', () => { expect(mockEnqueue).not.toHaveBeenCalled() }) }) + +describe('bounded dispatch', () => { + const options = { + limits: { workflowLogs: 7 }, + batchSize: 2, + dryRun: false, + requestId: 'wave-one', + } + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + setEnvFlags({ isBillingEnabled: false, isDataRetentionEnabled: true }) + mockIsTriggerAvailable.mockReturnValue(true) + mockTrigger.mockResolvedValue({ id: 'run-one' }) + mockRetrieve.mockResolvedValue({ payload: { mode: 'bounded', options } }) + }) + it('enqueues one run with seven-day idempotency, no retries, and a hard deadline', async () => { + const result = await dispatchBoundedCleanup('cleanup-logs', options) + expect(result).toMatchObject({ runId: 'run-one', limits: { workflowLogs: 7 } }) + expect(mockTrigger).toHaveBeenCalledTimes(1) + expect(mockTrigger).toHaveBeenCalledWith( + 'cleanup-logs', + expect.objectContaining({ mode: 'bounded' }), + expect.objectContaining({ + idempotencyKey: 'cleanup-logs:wave-one', + idempotencyKeyTTL: '7d', + maxAttempts: 1, + maxDuration: 180, + }) + ) + expect(mockTrigger.mock.calls[0][2]).not.toHaveProperty('concurrencyKey') + expect(mockBatchTrigger).not.toHaveBeenCalled() + expect(dbChainMockFns.select).not.toHaveBeenCalled() + }) + it('returns original budgets when the same request ID is retried with changed options', async () => { + const result = await dispatchBoundedCleanup('cleanup-logs', { + ...options, + limits: { workflowLogs: 50 }, + }) + expect(result.limits.workflowLogs).toBe(7) + expect(mockRetrieve).toHaveBeenCalledWith('run-one') + }) + it('fails without a fallback when Trigger is unavailable', async () => { + mockIsTriggerAvailable.mockReturnValue(false) + await expect(dispatchBoundedCleanup('cleanup-logs', options)).rejects.toThrow( + 'requires Trigger' + ) + expect(mockEnqueue).not.toHaveBeenCalled() + expect(mockTrigger).not.toHaveBeenCalled() + }) + it('preserves the data retention feature gate', async () => { + setEnvFlags({ isDataRetentionEnabled: false }) + await expect(dispatchBoundedCleanup('cleanup-logs', options)).rejects.toThrow('disabled') + expect(mockTrigger).not.toHaveBeenCalled() + }) + it('stops scope enumeration before another owner when the budget is exhausted', async () => { + queueTableRows( + schemaMock.workspace, + ['one', 'two'].map((id) => ({ id, organizationSettings: { logRetentionHours: 48 } })) + ) + let calls = 0 + await forEachCleanupChunk( + 'cleanup-logs', + async () => { + calls++ + }, + { strict: true, shouldStop: () => calls === 1 } + ) + expect(calls).toBe(1) + expect(dbChainMockFns.select).toHaveBeenCalledTimes(1) + }) + it('propagates bounded organization plan failures', async () => { + setEnvFlags({ isBillingEnabled: true }) + queueTableRows(schemaMock.workspace, []) + queueTableRows(schemaMock.organization, [ + { id: 'org-one', settings: { softDeleteRetentionHours: 24 } }, + ]) + mockGetOrganizationSubscription.mockRejectedValue(new Error('payer lookup failed')) + await expect( + forEachCleanupChunk('cleanup-soft-deletes', async () => {}, { strict: true }) + ).rejects.toThrow('payer lookup failed') + }) + it('does not create separate queues for scheduled logs and soft deletes', async () => { + mockBatchTrigger.mockResolvedValue({ batchId: 'batch-one' }) + for (const jobType of ['cleanup-logs', 'cleanup-soft-deletes'] as const) { + queueTableRows(schemaMock.workspace, [ + { + id: 'one', + organizationSettings: { logRetentionHours: 24, softDeleteRetentionHours: 24 }, + }, + ]) + queueTableRows(schemaMock.workspace, []) + await dispatchCleanupJobs(jobType) + } + expect(mockBatchTrigger).toHaveBeenCalledTimes(2) + for (const [, jobs] of mockBatchTrigger.mock.calls) + expect(jobs[0].options.concurrencyKey).toBeUndefined() + }) +}) diff --git a/apps/sim/lib/billing/cleanup-dispatcher.ts b/apps/sim/lib/billing/cleanup-dispatcher.ts index d70bc439369..aa9c3ed7505 100644 --- a/apps/sim/lib/billing/cleanup-dispatcher.ts +++ b/apps/sim/lib/billing/cleanup-dispatcher.ts @@ -3,12 +3,21 @@ import type { DataRetentionSettings, WorkspaceMode } from '@sim/db/schema' import { organization, workspace } from '@sim/db/schema' import { createLogger } from '@sim/logger' import { chunkArray } from '@sim/utils/helpers' -import { tasks } from '@trigger.dev/sdk' +import { runs, tasks } from '@trigger.dev/sdk' import { and, asc, eq, gt, isNull } from 'drizzle-orm' +import { validateBoundedCleanupOptions } from '@/lib/api/contracts/cleanup' import { getOrganizationSubscription } from '@/lib/billing/core/billing' import { getHighestPriorityPersonalSubscription } from '@/lib/billing/core/subscription' import { getPlanType, type PlanCategory } from '@/lib/billing/plan-helpers' import { type RetentionHoursKey, resolveEffectiveRetentionHours } from '@/lib/billing/retention' +import type { CleanupQuery, CleanupTransaction } from '@/lib/cleanup/bounded' +import { + type BoundedCleanupJobType, + type BoundedCleanupOptions, + type BoundedCleanupPayload, + LOG_CLEANUP_TYPES, + SOFT_DELETE_CLEANUP_TYPES, +} from '@/lib/cleanup/bounded-types' import { getJobQueue } from '@/lib/core/async-jobs' import { shouldExecuteInline } from '@/lib/core/async-jobs/config' import { resolveTriggerRegion } from '@/lib/core/async-jobs/region' @@ -60,8 +69,8 @@ const DAY = 24 type PlanResolutionEntry = readonly [string, PlanCategory] -function getCleanupConcurrencyKey(jobType: CleanupJobType): string { - return `cleanup:${jobType}` +function getCleanupConcurrencyKey(jobType: CleanupJobType): string | undefined { + return jobType === 'cleanup-tasks' ? `cleanup:${jobType}` : undefined } /** @@ -86,9 +95,10 @@ export const CLEANUP_CONFIG = { } as const satisfies Record async function listActiveWorkspaceCleanupScopeRowsPage( - afterId: string | null + afterId: string | null, + executor: Pick = db ): Promise { - const rows = await db + const rows = await executor .select({ id: workspace.id, billedAccountUserId: workspace.billedAccountUserId, @@ -113,31 +123,34 @@ async function listActiveWorkspaceCleanupScopeRowsPage( } async function resolvePersonalPlanTypesByBilledUserId( - rows: WorkspaceCleanupScopeRow[] + rows: WorkspaceCleanupScopeRow[], + options: CleanupScopeOptions = {} ): Promise> { const billedUserIds = Array.from(new Set(rows.map((row) => row.billedAccountUserId))) - const entries = await Promise.all( - billedUserIds.map(async (userId) => { - try { - const subscription = await getHighestPriorityPersonalSubscription(userId, { - onError: 'throw', - }) - return [userId, getPlanType(subscription?.plan)] as const - } catch (error) { - logger.error('Skipping cleanup for billed user after plan lookup failed', { - userId, - error, - }) - return null - } - }) - ) + const entries = await mapCleanupScopes(billedUserIds, options, async (userId) => { + try { + const subscription = await (options.query + ? options.query((executor) => + getHighestPriorityPersonalSubscription(userId, { onError: 'throw', executor }) + ) + : getHighestPriorityPersonalSubscription(userId, { onError: 'throw' })) + return [userId, getPlanType(subscription?.plan)] as const + } catch (error) { + if (options.strict) throw error + logger.error('Skipping cleanup for billed user after plan lookup failed', { + userId, + error, + }) + return null + } + }) return new Map(entries.filter((entry): entry is PlanResolutionEntry => entry !== null)) } async function resolvePlanTypesByWorkspaceId( - rows: WorkspaceCleanupScopeRow[] + rows: WorkspaceCleanupScopeRow[], + options: CleanupScopeOptions = {} ): Promise> { /** * Without billing there are no subscription rows to read, and the per-plan @@ -156,50 +169,55 @@ async function resolvePlanTypesByWorkspaceId( } const userScopedRows = rows.filter((row) => row.workspaceMode !== WORKSPACE_MODE.ORGANIZATION) - const userPlanByBilledUserId = await resolvePersonalPlanTypesByBilledUserId(userScopedRows) - const entries = await Promise.all( - rows.map(async (row) => { - if (row.workspaceMode === WORKSPACE_MODE.ORGANIZATION) { - const organizationId = isOrganizationWorkspace(row) ? row.organizationId : null - if (!organizationId) { - logger.error('Skipping cleanup for malformed organization workspace', { - workspaceId: row.id, - organizationId: row.organizationId, - }) - return null - } - - try { - const subscription = await getOrganizationSubscription(organizationId, { - onError: 'throw', - }) - if (!subscription) { - logger.warn('Skipping cleanup for organization workspace without an org subscription', { - workspaceId: row.id, - organizationId, - }) - return null - } + const userPlanByBilledUserId = await resolvePersonalPlanTypesByBilledUserId( + userScopedRows, + options + ) + const entries = await mapCleanupScopes(rows, options, async (row) => { + if (row.workspaceMode === WORKSPACE_MODE.ORGANIZATION) { + const organizationId = isOrganizationWorkspace(row) ? row.organizationId : null + if (!organizationId) { + if (options.strict) throw new Error(`Malformed organization workspace ${row.id}`) + logger.error('Skipping cleanup for malformed organization workspace', { + workspaceId: row.id, + organizationId: row.organizationId, + }) + return null + } - return [row.id, getPlanType(subscription?.plan)] as const - } catch (error) { - logger.error('Skipping cleanup for organization workspace after plan lookup failed', { + try { + const subscription = await (options.query + ? options.query((executor) => + getOrganizationSubscription(organizationId, { onError: 'throw', executor }) + ) + : getOrganizationSubscription(organizationId, { onError: 'throw' })) + if (!subscription) { + logger.warn('Skipping cleanup for organization workspace without an org subscription', { workspaceId: row.id, organizationId, - error, }) return null } - } - const plan = userPlanByBilledUserId.get(row.billedAccountUserId) - if (plan === undefined) { + return [row.id, getPlanType(subscription?.plan)] as const + } catch (error) { + if (options.strict) throw error + logger.error('Skipping cleanup for organization workspace after plan lookup failed', { + workspaceId: row.id, + organizationId, + error, + }) return null } + } - return [row.id, plan] as const - }) - ) + const plan = userPlanByBilledUserId.get(row.billedAccountUserId) + if (plan === undefined) { + return null + } + + return [row.id, plan] as const + }) return new Map(entries.filter((entry): entry is PlanResolutionEntry => entry !== null)) } @@ -223,9 +241,16 @@ const GLOBAL_HOUSEKEEPING_PLAN: Partial> = 'cleanup-logs': 'free', } -async function forEachCleanupChunk( +type CleanupScopeOptions = { + shouldStop?: () => boolean + strict?: boolean + query?: CleanupQuery +} + +export async function forEachCleanupChunk( jobType: CleanupJobType, - onChunk: (payload: CleanupJobPayload) => Promise + onChunk: (payload: CleanupJobPayload) => Promise, + options: CleanupScopeOptions = {} ): Promise<{ chunkCount: number; workspaceCount: number }> { const config = CLEANUP_CONFIG[jobType] const chunkCountByPlan: Partial> = {} @@ -236,6 +261,7 @@ async function forEachCleanupChunk( let afterId: string | null = null const emitChunk = async (payload: CleanupJobPayload) => { + if (options.shouldStop?.()) return if (payload.plan === housekeepingPlan && !housekeepingAssigned) { payload.runGlobalHousekeeping = true housekeepingAssigned = true @@ -244,12 +270,14 @@ async function forEachCleanupChunk( await onChunk(payload) } - while (true) { - const rows = await listActiveWorkspaceCleanupScopeRowsPage(afterId) + while (!options.shouldStop?.()) { + const rows: WorkspaceCleanupScopeRow[] = await (options.query + ? options.query((tx) => listActiveWorkspaceCleanupScopeRowsPage(afterId, tx)) + : listActiveWorkspaceCleanupScopeRowsPage(afterId)) if (rows.length === 0) break afterId = rows[rows.length - 1].id - const planByWorkspaceId = await resolvePlanTypesByWorkspaceId(rows) + const planByWorkspaceId = await resolvePlanTypesByWorkspaceId(rows, options) for (const plan of NON_ENTERPRISE_PLANS) { const retentionHours = config.defaults[plan] @@ -275,6 +303,7 @@ async function forEachCleanupChunk( } for (const row of rows) { + if (options.shouldStop?.()) break if (planByWorkspaceId.get(row.id) !== 'enterprise') continue const hours = resolveEffectiveRetentionHours({ orgSettings: row.organizationSettings, @@ -294,23 +323,33 @@ async function forEachCleanupChunk( if (jobType === 'cleanup-soft-deletes' || jobType === 'cleanup-tasks') { let afterOrganizationId: string | null = null - while (true) { - const organizations = await db - .select({ id: organization.id, settings: organization.dataRetentionSettings }) - .from(organization) - .where(afterOrganizationId ? gt(organization.id, afterOrganizationId) : undefined) - .orderBy(asc(organization.id)) - .limit(WORKSPACE_SCOPE_PAGE_SIZE) + while (!options.shouldStop?.()) { + const selectOrganizations = async (executor: Pick) => + executor + .select({ id: organization.id, settings: organization.dataRetentionSettings }) + .from(organization) + .where(afterOrganizationId ? gt(organization.id, afterOrganizationId) : undefined) + .orderBy(asc(organization.id)) + .limit(WORKSPACE_SCOPE_PAGE_SIZE) + const organizations = await (options.query + ? options.query(selectOrganizations) + : selectOrganizations(db)) if (organizations.length === 0) break afterOrganizationId = organizations[organizations.length - 1].id for (const row of organizations) { + if (options.shouldStop?.()) break let plan: PlanCategory = 'enterprise' if (isBillingEnabled) { try { - const subscription = await getOrganizationSubscription(row.id, { onError: 'throw' }) + const subscription = await (options.query + ? options.query((executor) => + getOrganizationSubscription(row.id, { onError: 'throw', executor }) + ) + : getOrganizationSubscription(row.id, { onError: 'throw' })) if (!subscription) continue plan = getPlanType(subscription.plan) } catch (error) { + if (options.strict) throw error logger.error('Skipping organization cleanup after plan lookup failed', { organizationId: row.id, error, @@ -463,3 +502,50 @@ export async function dispatchCleanupJobs(jobType: CleanupJobType): Promise<{ return { jobIds, jobCount: jobIds.length, chunkCount, workspaceCount } } + +/** One idempotent, sequential maintenance run; no inline or queue fallback. */ +export async function dispatchBoundedCleanup( + jobType: BoundedCleanupJobType, + input: BoundedCleanupOptions +) { + if (!isBillingEnabled && !isDataRetentionEnabled) throw new Error('Data retention is disabled') + if (!isTriggerAvailable()) throw new Error('Bounded cleanup requires Trigger.dev') + const types = jobType === 'cleanup-logs' ? LOG_CLEANUP_TYPES : SOFT_DELETE_CLEANUP_TYPES + const options = validateBoundedCleanupOptions(input, types) + const payload: BoundedCleanupPayload = { mode: 'bounded', options } + const handle = await tasks.trigger(jobType, payload, { + idempotencyKey: `${jobType}:${options.requestId}`, + idempotencyKeyTTL: '7d', + maxAttempts: 1, + maxDuration: 180, + tags: [`jobType:${jobType}`, 'cleanup:bounded'], + region: await resolveTriggerRegion(), + }) + // The same requestId may have been submitted earlier with different options. + // Return the original accepted payload rather than claiming the new limits applied. + const run = await runs.retrieve(handle.id) + const accepted = run.payload as BoundedCleanupPayload | undefined + if (accepted?.mode !== 'bounded' || !accepted.options) + throw new Error('Missing bounded cleanup run payload') + return { + triggered: true, + mode: 'bounded' as const, + runId: handle.id, + ...validateBoundedCleanupOptions(accepted.options, types), + } +} + +/** Bounded maintenance resolves payers serially instead of queuing hundreds of reads. */ +async function mapCleanupScopes( + rows: T[], + options: CleanupScopeOptions, + map: (row: T) => Promise +): Promise { + if (!options.strict) return Promise.all(rows.map(map)) + const result: R[] = [] + for (const row of rows) { + if (options.shouldStop?.()) break + result.push(await map(row)) + } + return result +} diff --git a/apps/sim/lib/billing/core/billing-attribution.ts b/apps/sim/lib/billing/core/billing-attribution.ts index 9b996b6ad83..68bc461c3c0 100644 --- a/apps/sim/lib/billing/core/billing-attribution.ts +++ b/apps/sim/lib/billing/core/billing-attribution.ts @@ -104,6 +104,7 @@ export interface ResolveBillingAttributionParams { } export interface ResolveWorkspaceBillingPayerOptions { + executor?: typeof db | Parameters[0]>[0] onMissing?: 'throw' | 'return-null' } @@ -655,7 +656,8 @@ export async function resolveWorkspaceBillingPayer( workspaceId: string, options: ResolveWorkspaceBillingPayerOptions = {} ) { - const [workspacePayer] = await db + const executor = options.executor ?? db + const [workspacePayer] = await executor .select({ billedAccountUserId: workspace.billedAccountUserId, organizationId: workspace.organizationId, @@ -670,9 +672,10 @@ export async function resolveWorkspaceBillingPayer( } const { billedAccountUserId, organizationId } = workspacePayer + const readOptions = { onError: 'throw' as const, ...(options.executor ? { executor } : {}) } const payerSubscription = organizationId - ? await getOrganizationSubscription(organizationId, { onError: 'throw' }) - : await getHighestPriorityPersonalSubscription(billedAccountUserId, { onError: 'throw' }) + ? await getOrganizationSubscription(organizationId, readOptions) + : await getHighestPriorityPersonalSubscription(billedAccountUserId, readOptions) const expectedReferenceId = organizationId ?? billedAccountUserId if (payerSubscription && payerSubscription.referenceId !== expectedReferenceId) { diff --git a/apps/sim/lib/billing/storage/context.ts b/apps/sim/lib/billing/storage/context.ts index f6dc7b95cc8..b554a417c91 100644 --- a/apps/sim/lib/billing/storage/context.ts +++ b/apps/sim/lib/billing/storage/context.ts @@ -1,5 +1,8 @@ import { isRecordLike } from '@sim/utils/object' -import { resolveWorkspaceBillingPayer } from '@/lib/billing/core/billing-attribution' +import { + type ResolveWorkspaceBillingPayerOptions, + resolveWorkspaceBillingPayer, +} from '@/lib/billing/core/billing-attribution' import type { BillingEntity } from '@/lib/billing/core/usage-log' /** @@ -25,9 +28,10 @@ function readCustomStorageLimitGB(metadata: unknown): number | null { * the uploader's subscriptions or organization memberships. */ export async function resolveStorageBillingContext( - workspaceId: string + workspaceId: string, + options: ResolveWorkspaceBillingPayerOptions = {} ): Promise { - const payer = await resolveWorkspaceBillingPayer(workspaceId) + const payer = await resolveWorkspaceBillingPayer(workspaceId, options) if (!payer) { throw new Error(`Unable to resolve storage payer for workspace ${workspaceId}`) } diff --git a/apps/sim/lib/cleanup/bounded-cleanup.md b/apps/sim/lib/cleanup/bounded-cleanup.md new file mode 100644 index 00000000000..78963fb17f1 --- /dev/null +++ b/apps/sim/lib/cleanup/bounded-cleanup.md @@ -0,0 +1,75 @@ +# Gradually draining retention cleanup + +Use the existing authenticated GET endpoints through the cron Lambda. These parameters select a bounded run; the HTTP response is **202 Accepted**, not completion. It includes `runId` and the original accepted options. Inspect the Trigger.dev run and its `cleanup` metadata before invoking again. + +## Invocation + +First preview a small log batch with the existing Lambda event shape: + +```json +{"path":"/api/logs/cleanup?workflowLogs=100&jobLogs=100&batchSize=25&dryRun=true&requestId=logs-preview-001"} +``` + +Then delete a small batch, using a new request ID: + +```json +{"path":"/api/logs/cleanup?workflowLogs=100&jobLogs=100&batchSize=25&requestId=logs-delete-001"} +``` + +Start soft-delete parents smaller because they may have many children: + +```json +{"path":"/api/cron/cleanup-soft-deletes?workflows=2&chats=5&legacyFiles=10&files=10&batchSize=1&dryRun=true&requestId=soft-preview-001"} +``` + +The Lambda already supplies the cron-secret authorization header. Direct HTTP calls use the same `Authorization: Bearer ` header. Query parameters live inside `path`; no Lambda code change is needed. + +### Limits + +Each value is an integer from **0 to 5000**, shared across every workspace and organization in that run. Unspecified types have a zero budget. At least one limit must be positive. + +| Endpoint | Limit parameters | +| --- | --- | +| `/api/logs/cleanup` | `workflowLogs`, `jobLogs`, `largeValues`, `legacyLargeValues`, `orphanSnapshots`, `staleReferences`, `staleDependencies`, `largeValueTombstones` | +| `/api/cron/cleanup-soft-deletes` | `workflows`, `chats`, `legacyFiles`, `files`, `knowledgeBases`, `folders`, `userTables`, `memories`, `mcpServers`, `workflowMcpServers`, `orphanKnowledgeBaseBindings` | + +- `batchSize`: 1–500, default 25. Maximum root selections per batch; some operations use smaller statements. +- `dryRun`: `true` or `false`, default `false`. Counts distinct eligible roots without writes, storage deletion, billing changes, or backend calls. +- `requestId`: required, 1–128 letters/digits/underscores/hyphens. Reusing the same ID on the same endpoint returns the original run for seven days, even if parameters changed. Use a **new ID for every intended batch**, including switching from preview to deletion. After seven days, an old ID can start another run. +- Unknown, duplicate, blank, fractional, negative, or out-of-range parameters are rejected. Controls without a positive limit are rejected. + +A call with **no query parameters retains the existing scheduled cleanup behavior**, including its larger budgets. Existing retention windows, enterprise overrides, paused execution/reference protection, and the retention feature gate still apply. There is no bounded change to Copilot task retention. + +## What the budget means + +Limits count **selected roots**, including roots restored before deletion. They do not count all physical rows affected by foreign keys. Two workflows can cascade into thousands of blocks, edges, chats, and messages. Mandatory attached-file and backend cleanup follows selected parents even when its standalone type budget is zero. Knowledge-base deletion retains the existing document accounting and storage-cleanup outbox behavior. + +`largeValues`/`legacyLargeValues` count object keys. `orphanKnowledgeBaseBindings` counts bindings soft-deleted after object cleanup. Metadata pruning counts its selected metadata records. A dry run previews the current state; it does not reserve rows for a later deletion run. + +## Run controls and results + +- One coordinator walks owner scopes sequentially with shared budgets. It creates no child cleanup jobs. +- Logs and soft deletes share the named Trigger queue `retention-cleanup`, concurrency 1, including newly dispatched scheduled jobs. No per-type concurrency keys are used. +- Bounded dispatch sets one attempt and a 180-second hard maximum. The worker stops starting new root batches after 120 seconds; cancellable child preparation also observes that work deadline. +- Cleanup SQL uses transaction-local **500ms lock_timeout** and **5s statement_timeout**. The billable-file delete and storage decrement remain atomic. +- Trigger `cleanup` metadata reports each requested type's `selected`, `deleted`, `skipped`, `filesDeleted`, and `filesFailed`, plus stage, duration, and stop reason: `budgets_exhausted`, `scopes_exhausted`, `time_budget`, or `failed`. +- A failure stops subsequent stages, preserves completed progress, and fails the run. External deletion is not transactional with Postgres. A hard process termination may leave the last metadata checkpoint behind actual effects. Do not blindly replay failed runs: inspect the failed stage and any external work already completed. This change does not add recovery for the existing gap when chat backend/storage cleanup fails after parent rows have committed. + +## Rollout + +1. Deploy Trigger workers first, then the API. Keep the existing production cleanup schedules disabled. +2. Let old cleanup runs finish or cancel them before manual draining. Old queued jobs may retain their previous queue/version settings; the new queue cannot serialize against those runs. +3. Run a preview, then one small delete invocation. Wait for the Trigger run to finish. Check database CPU, query latency, lock waits, replication lag, and application errors against their normal baseline. +4. Repeat with a fresh request ID. Increase either the per-call budget or call frequency gradually, holding the other steady. Pause submissions on timeouts, failures, or database/application degradation. Avoid overlapping caller loops even though the Trigger queue serializes these jobs. +5. Once backlog and load are stable, re-enable schedules with **explicit bounded query parameters** and a fresh request ID per scheduled invocation. A fixed path containing a fixed ID will deduplicate for seven days; an unparameterized path uses the old larger cleanup behavior. Updating dynamic scheduled payloads is a separate rollout action. + +## Validation + +From `apps/sim`, run the bounded unit and route tests with Vitest. The isolated PostgreSQL suite deliberately skips app environment files and DB mocks: + +```sh +DATABASE_URL=postgresql://bounded_cleanup@127.0.0.1:55439/bounded_cleanup_test \ + bunx vitest run --config lib/cleanup/vitest.postgres.config.ts +``` + +Use a disposable local database named `bounded_cleanup_test`. The suite creates and removes `cleanup_fixture_*` and minimal execution-retention fixture tables in that disposable database, and verifies cross-owner budgets, dry-run safety, cascades, restore races, partial commits, and local timeout settings. diff --git a/apps/sim/lib/cleanup/bounded-delete.ts b/apps/sim/lib/cleanup/bounded-delete.ts new file mode 100644 index 00000000000..51af402f223 --- /dev/null +++ b/apps/sim/lib/cleanup/bounded-delete.ts @@ -0,0 +1,43 @@ +import { and, inArray, notInArray, type SQL } from 'drizzle-orm' +import type { PgColumn, PgTable } from 'drizzle-orm/pg-core' +import type { BoundedCleanup } from '@/lib/cleanup/bounded' +import type { CleanupType } from '@/lib/cleanup/bounded-types' + +/** Select and delete the same roots, reasserting eligibility after side effects. */ +export async function boundedDelete( + control: BoundedCleanup, + type: CleanupType, + table: PgTable, + id: PgColumn, + eligibility: SQL | undefined, + hooks: { + before?: (ids: string[]) => Promise + after?: (ids: string[]) => Promise + } = {} +) { + if (!eligibility) throw new Error(`Missing ${type} cleanup eligibility`) + await control.batches( + type, + (limit, seen) => + control.query(async (tx) => + tx + .select({ id }) + .from(table) + .where(and(eligibility, seen.length ? notInArray(id, seen) : undefined)) + .limit(limit) + ) as Promise<{ id: string }[]>, + (row) => row.id, + async (rows) => { + const ids = rows.map((row) => row.id) + await hooks.before?.(ids) + const deleted = await control.query(async (tx) => + tx + .delete(table) + .where(and(inArray(id, ids), eligibility)) + .returning({ id }) + ) + await control.deleted(type, deleted.length) + await hooks.after?.(ids) + } + ) +} diff --git a/apps/sim/lib/cleanup/bounded-large-value-metadata.ts b/apps/sim/lib/cleanup/bounded-large-value-metadata.ts new file mode 100644 index 00000000000..87aedbad279 --- /dev/null +++ b/apps/sim/lib/cleanup/bounded-large-value-metadata.ts @@ -0,0 +1,82 @@ +import { + executionLargeValueDependencies, + executionLargeValueReferences, + executionLargeValues, +} from '@sim/db/schema' +import { type SQL, sql } from 'drizzle-orm' +import type { BoundedCleanup } from '@/lib/cleanup/bounded' +import type { CleanupType } from '@/lib/cleanup/bounded-types' +import { + largeValueTombstonePredicate, + staleLargeValueDependencyPredicate, + staleLargeValueReferencePredicate, +} from '@/lib/execution/payloads/large-value-metadata' + +/** Stable primary-key identities survive updates between selection and deletion; never retain ctid across transactions. */ + +export async function pruneBoundedLargeValueMetadata( + control: BoundedCleanup, + workspaceIds: string[] +) { + if (workspaceIds.length === 0) return + const targets: { + type: CleanupType + from: SQL + identity: SQL + predicate: SQL + exact: (id: string) => SQL + }[] = [ + { + type: 'staleReferences', + exact: (id) => { + const [key, executionId, source] = JSON.parse(id) as [string, string, string] + return sql`ref.key = ${key} AND ref.execution_id = ${executionId} AND ref.source = ${source}` + }, + from: sql`${executionLargeValueReferences} AS ref`, + identity: sql`jsonb_build_array(ref.key, ref.execution_id, ref.source)::text`, + predicate: sql`ref.workspace_id IN ${workspaceIds} AND ${staleLargeValueReferencePredicate()}`, + }, + { + type: 'staleDependencies', + exact: (id) => { + const [parentKey, childKey] = JSON.parse(id) as [string, string] + return sql`dependency.parent_key = ${parentKey} AND dependency.child_key = ${childKey}` + }, + from: sql`${executionLargeValueDependencies} AS dependency`, + identity: sql`jsonb_build_array(dependency.parent_key, dependency.child_key)::text`, + predicate: sql`dependency.workspace_id IN ${workspaceIds} AND ${staleLargeValueDependencyPredicate()}`, + }, + { + type: 'largeValueTombstones', + exact: (id) => sql`value.key = ${id}`, + from: sql`${executionLargeValues} AS value`, + identity: sql`value.key`, + predicate: sql`value.workspace_id IN ${workspaceIds} AND ${largeValueTombstonePredicate(new Date(Date.now() - 30 * 86400_000))}`, + }, + ] + for (const target of targets) { + await control.batches( + target.type, + (limit, seen) => + control.query(async (tx) => { + const rows = await tx.execute<{ + id: string + }>(sql`SELECT ${target.identity} AS id FROM ${target.from} + WHERE ${target.predicate} ${seen.length ? sql`AND ${target.identity} NOT IN ${seen}` : sql``} + LIMIT ${limit}`) + return [...rows] + }), + (row) => row.id, + async (rows) => { + const deleted = await control.query(async (tx) => + tx.execute(sql`DELETE FROM ${target.from} + WHERE ${target.predicate} AND (${sql.join( + rows.map((row) => sql`(${target.exact(row.id)})`), + sql` OR ` + )}) RETURNING ${target.identity}`) + ) + await control.deleted(target.type, deleted.length) + } + ) + } +} diff --git a/apps/sim/lib/cleanup/bounded-runner.ts b/apps/sim/lib/cleanup/bounded-runner.ts new file mode 100644 index 00000000000..b96622de578 --- /dev/null +++ b/apps/sim/lib/cleanup/bounded-runner.ts @@ -0,0 +1,49 @@ +import { metadata } from '@trigger.dev/sdk' +import { validateBoundedCleanupOptions } from '@/lib/api/contracts/cleanup' +import { type CleanupJobPayload, forEachCleanupChunk } from '@/lib/billing/cleanup-dispatcher' +import { BoundedCleanup, CleanupTimeBudgetReached } from '@/lib/cleanup/bounded' +import { + type BoundedCleanupJobType, + type BoundedCleanupPayload, + LOG_CLEANUP_TYPES, + SOFT_DELETE_CLEANUP_TYPES, +} from '@/lib/cleanup/bounded-types' +import { isBillingEnabled, isDataRetentionEnabled } from '@/lib/core/config/env-flags' + +/** Execute scopes in this run; never fan out budgets into child jobs. */ +export async function runBoundedCleanup( + jobType: BoundedCleanupJobType, + payload: BoundedCleanupPayload, + runScope: (payload: CleanupJobPayload, control: BoundedCleanup) => Promise +) { + const options = validateBoundedCleanupOptions( + payload.options, + jobType === 'cleanup-logs' ? LOG_CLEANUP_TYPES : SOFT_DELETE_CLEANUP_TYPES + ) + const control = new BoundedCleanup(options, async (progress) => { + metadata.set('cleanup', progress) + await metadata.flush() + }) + try { + if (!isBillingEnabled && !isDataRetentionEnabled) throw new Error('Data retention is disabled') + await control.checkpoint() + await forEachCleanupChunk( + jobType, + async (scope) => { + await runScope(scope, control) + control.progress.stage = 'scopes' + await control.checkpoint() + }, + { + shouldStop: () => control.stopped(), + strict: true, + query: control.query, + } + ) + return await control.finish() + } catch (error) { + if (error instanceof CleanupTimeBudgetReached) return control.finish() + await control.fail(error) + throw error + } +} diff --git a/apps/sim/lib/cleanup/bounded-storage.ts b/apps/sim/lib/cleanup/bounded-storage.ts new file mode 100644 index 00000000000..bb5da33a679 --- /dev/null +++ b/apps/sim/lib/cleanup/bounded-storage.ts @@ -0,0 +1,41 @@ +import { workspaceFiles } from '@sim/db/schema' +import { chunkArray } from '@sim/utils/helpers' +import { and, inArray, isNull } from 'drizzle-orm' +import type { BoundedCleanup } from '@/lib/cleanup/bounded' +import type { CleanupType } from '@/lib/cleanup/bounded-types' +import { isUsingCloudStorage, type StorageContext, StorageService } from '@/lib/uploads' + +/** Storage failures stop the run; progress includes objects already removed. */ +export async function deleteBoundedStorage( + control: BoundedCleanup, + type: CleanupType, + keys: string[], + context: StorageContext +) { + if (!isUsingCloudStorage()) return + for (const batch of chunkArray([...new Set(keys)], control.options.batchSize)) { + let result: Awaited> + try { + result = await StorageService.deleteFiles(batch, context) + } catch (error) { + await control.files(type, 0, batch.length) + throw error + } + await control.files(type, result.deleted, result.failed.length) + if (result.failed.length > 0) + throw new Error(`${type}: ${result.failed.length} storage deletions failed`) + } +} + +/** Equivalent to deleteFileMetadata, on the cleanup pool with local timeouts. */ +export async function tombstoneBoundedFiles(control: BoundedCleanup, keys: string[]) { + if (keys.length === 0) return + for (const batch of chunkArray(keys, control.options.batchSize)) { + await control.query(async (tx) => { + await tx + .update(workspaceFiles) + .set({ deletedAt: new Date() }) + .where(and(inArray(workspaceFiles.key, batch), isNull(workspaceFiles.deletedAt))) + }) + } +} diff --git a/apps/sim/lib/cleanup/bounded-types.ts b/apps/sim/lib/cleanup/bounded-types.ts new file mode 100644 index 00000000000..5bc59b30f01 --- /dev/null +++ b/apps/sim/lib/cleanup/bounded-types.ts @@ -0,0 +1,53 @@ +/** Root budgets are shared by all owner scopes in one invocation, including dry runs. */ +export const LOG_CLEANUP_TYPES = [ + 'workflowLogs', + 'jobLogs', + 'largeValues', + 'legacyLargeValues', + 'orphanSnapshots', + 'staleReferences', + 'staleDependencies', + 'largeValueTombstones', +] as const +export const SOFT_DELETE_CLEANUP_TYPES = [ + 'workflows', + 'chats', + 'legacyFiles', + 'files', + 'knowledgeBases', + 'folders', + 'userTables', + 'memories', + 'mcpServers', + 'workflowMcpServers', + 'orphanKnowledgeBaseBindings', +] as const +export type CleanupType = + | (typeof LOG_CLEANUP_TYPES)[number] + | (typeof SOFT_DELETE_CLEANUP_TYPES)[number] +export type BoundedCleanupJobType = 'cleanup-logs' | 'cleanup-soft-deletes' +export type BoundedCleanupOptions = { + limits: Partial> + batchSize: number + dryRun: boolean + requestId: string +} +export type BoundedCleanupPayload = { mode: 'bounded'; options: BoundedCleanupOptions } +export type CleanupStageProgress = { + selected: number + deleted: number + skipped: number + filesDeleted: number + filesFailed: number +} +export type CleanupProgress = { + requestId: string + dryRun: boolean + limits: BoundedCleanupOptions['limits'] + batchSize: number + stages: Partial> + stage: CleanupType | 'scopes' + durationMs: number + stopReason?: 'budgets_exhausted' | 'scopes_exhausted' | 'time_budget' | 'failed' + error?: string +} diff --git a/apps/sim/lib/cleanup/bounded.postgres.integration.ts b/apps/sim/lib/cleanup/bounded.postgres.integration.ts new file mode 100644 index 00000000000..f9bc37391d5 --- /dev/null +++ b/apps/sim/lib/cleanup/bounded.postgres.integration.ts @@ -0,0 +1,199 @@ +import { db, dbFor } from '@sim/db' +import { eq, sql } from 'drizzle-orm' +import { pgTable, text } from 'drizzle-orm/pg-core' +import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest' +import { BoundedCleanup, cleanupQuery } from '@/lib/cleanup/bounded' +import { boundedDelete } from '@/lib/cleanup/bounded-delete' +import { pruneBoundedLargeValueMetadata } from '@/lib/cleanup/bounded-large-value-metadata' + +const url = new URL(process.env.DATABASE_URL ?? '') +if (url.hostname !== '127.0.0.1' || url.pathname !== '/bounded_cleanup_test') { + throw new Error('This suite requires an isolated local bounded_cleanup_test database') +} +const roots = pgTable('cleanup_fixture_roots', { + id: text('id').primaryKey(), + owner: text('owner').notNull(), +}) +const client = dbFor('cleanup') + +beforeAll(async () => { + await db.execute( + sql`CREATE TABLE execution_large_value_references (key text, workspace_id text, execution_id text, source text, PRIMARY KEY(key, execution_id, source))` + ) + await db.execute( + sql`CREATE TABLE execution_large_value_dependencies (parent_key text, child_key text, workspace_id text, PRIMARY KEY(parent_key, child_key))` + ) + await db.execute( + sql`CREATE TABLE execution_large_values (key text PRIMARY KEY, workspace_id text, deleted_at timestamp)` + ) + await db.execute(sql`CREATE TABLE workflow_execution_logs (execution_id text)`) + await db.execute(sql`CREATE TABLE paused_executions (execution_id text, status text)`) + + await db.execute( + sql`CREATE TABLE cleanup_fixture_roots (id text PRIMARY KEY, owner text NOT NULL)` + ) + await db.execute( + sql`CREATE TABLE cleanup_fixture_children (id serial PRIMARY KEY, root_id text REFERENCES cleanup_fixture_roots(id) ON DELETE CASCADE)` + ) +}) +beforeEach(async () => { + await db.execute( + sql`TRUNCATE execution_large_value_references, execution_large_value_dependencies, execution_large_values, workflow_execution_logs, paused_executions` + ) + + await db.execute(sql`TRUNCATE cleanup_fixture_roots, cleanup_fixture_children`) + await db + .insert(roots) + .values( + ['a', 'b', 'c', 'd', 'e', 'f'].map((id, index) => ({ id, owner: index < 3 ? 'one' : 'two' })) + ) + await db.execute( + sql`INSERT INTO cleanup_fixture_children(root_id) SELECT id FROM cleanup_fixture_roots CROSS JOIN generate_series(1,20)` + ) +}) +afterAll(async () => { + await db.execute( + sql`DROP TABLE IF EXISTS execution_large_value_references, execution_large_value_dependencies, execution_large_values, workflow_execution_logs, paused_executions` + ) + + await db.execute(sql`DROP TABLE IF EXISTS cleanup_fixture_children, cleanup_fixture_roots`) + await client.$client.end() + await db.$client.end() +}) + +function control(limit: number, dryRun = false) { + return new BoundedCleanup( + { limits: { workflows: limit }, batchSize: 2, requestId: 'pg', dryRun }, + async () => {} + ) +} +async function count(table: 'roots' | 'children') { + const rows = await db.execute<{ count: number }>( + sql`SELECT count(*)::int AS count FROM ${sql.identifier(`cleanup_fixture_${table}`)}` + ) + return rows[0].count +} + +describe('bounded cleanup against PostgreSQL', () => { + it('uses one budget across owner scopes with small committed batches', async () => { + const run = control(5) + await boundedDelete(run, 'workflows', roots, roots.id, eq(roots.owner, 'one')) + await boundedDelete(run, 'workflows', roots, roots.id, eq(roots.owner, 'two')) + expect(await count('roots')).toBe(1) + expect(run.progress.stages.workflows).toMatchObject({ selected: 5, deleted: 5 }) + }) + it('dry run counts distinct roots without touching roots or children', async () => { + const run = control(5, true) + await boundedDelete(run, 'workflows', roots, roots.id, sql`true`) + expect(run.progress.stages.workflows?.selected).toBe(5) + expect(await count('roots')).toBe(6) + expect(await count('children')).toBe(120) + }) + it('counts roots separately from cascaded child rows', async () => { + const run = control(1) + await boundedDelete(run, 'workflows', roots, roots.id, sql`true`) + expect(run.progress.stages.workflows?.deleted).toBe(1) + expect(await count('children')).toBe(100) + }) + it('charges and skips a root restored between selection and deletion', async () => { + const run = control(1) + await boundedDelete(run, 'workflows', roots, roots.id, eq(roots.owner, 'one'), { + before: async (ids) => { + await db.update(roots).set({ owner: 'restored' }).where(eq(roots.id, ids[0])) + }, + }) + expect(run.progress.stages.workflows).toMatchObject({ selected: 1, deleted: 0, skipped: 1 }) + expect(await count('roots')).toBe(6) + }) + it('aborts a blocked delete after the local lock timeout and preserves prior batches', async () => { + let signalLocked!: () => void + let release!: () => void + const locked = new Promise((resolve) => { + signalLocked = resolve + }) + const released = new Promise((resolve) => { + release = resolve + }) + const holding = db.transaction(async (tx) => { + await tx.execute(sql`SELECT id FROM cleanup_fixture_roots WHERE id = 'c' FOR UPDATE`) + signalLocked() + await released + }) + await locked + const run = control(5) + const started = Date.now() + try { + await expect(boundedDelete(run, 'workflows', roots, roots.id, sql`true`)).rejects.toThrow() + expect(Date.now() - started).toBeLessThan(3000) + expect(run.progress.stages.workflows).toMatchObject({ selected: 4, deleted: 2 }) + expect(await count('roots')).toBe(4) + } finally { + release() + await holding + } + }) + it('enforces the 5s statement timeout and does not leak settings to the next transaction', async () => { + const started = Date.now() + await expect(cleanupQuery(async (tx) => tx.execute(sql`SELECT pg_sleep(10)`))).rejects.toThrow() + expect(Date.now() - started).toBeGreaterThanOrEqual(4500) + expect(Date.now() - started).toBeLessThan(8000) + const settings = await client.execute<{ statement: string; lock: string }>( + sql`SELECT current_setting('statement_timeout') AS statement, current_setting('lock_timeout') AS lock` + ) + expect(settings[0]).toMatchObject({ statement: '0', lock: '0' }) + }) +}) + +describe('bounded metadata SQL against PostgreSQL', () => { + it('retains live log and paused references while bounding stale reference deletion', async () => { + await db.execute(sql`INSERT INTO workflow_execution_logs VALUES ('live-log')`) + await db.execute(sql`INSERT INTO paused_executions VALUES ('live-pause', 'paused')`) + await db.execute(sql`INSERT INTO execution_large_value_references VALUES + ('live-1','one','live-log','execution_log'), ('live-2','one','live-pause','paused_snapshot'), + ('stale-1','one','missing-1','execution_log'), ('stale-2','one','missing-2','execution_log'), + ('stale-3','one','missing-3','unknown')`) + const run = new BoundedCleanup( + { limits: { staleReferences: 2 }, batchSize: 1, requestId: 'refs', dryRun: false }, + async () => {} + ) + await pruneBoundedLargeValueMetadata(run, ['one']) + expect(run.progress.stages.staleReferences).toMatchObject({ selected: 2, deleted: 2 }) + const live = await db.execute( + sql`SELECT key FROM execution_large_value_references WHERE key LIKE 'live-%'` + ) + expect(live.length).toBe(2) + expect((await db.execute(sql`SELECT key FROM execution_large_value_references`)).length).toBe(3) + }) + it('previews distinct dependencies without deleting them', async () => { + await db.execute( + sql`INSERT INTO execution_large_value_dependencies VALUES ('gone-1','child','one'), ('gone-2','child','one'), ('gone-3','child','one')` + ) + const run = new BoundedCleanup( + { limits: { staleDependencies: 2 }, batchSize: 1, requestId: 'deps', dryRun: true }, + async () => {} + ) + await pruneBoundedLargeValueMetadata(run, ['one']) + expect(run.progress.stages.staleDependencies).toMatchObject({ selected: 2, deleted: 0 }) + expect( + (await db.execute(sql`SELECT parent_key FROM execution_large_value_dependencies`)).length + ).toBe(3) + }) + it('retains parent tombstones with dependencies and respects the 30-day grace period', async () => { + await db.execute(sql`INSERT INTO execution_large_values VALUES + ('protected','one',now() - interval '40 days'), ('old','one',now() - interval '40 days'), ('recent','one',now())`) + await db.execute( + sql`INSERT INTO execution_large_value_dependencies VALUES ('protected','child','one')` + ) + const run = new BoundedCleanup( + { limits: { largeValueTombstones: 5 }, batchSize: 1, requestId: 'tombs', dryRun: false }, + async () => {} + ) + await pruneBoundedLargeValueMetadata(run, ['one']) + expect(run.progress.stages.largeValueTombstones).toMatchObject({ selected: 1, deleted: 1 }) + expect( + ( + await db.execute<{ key: string }>(sql`SELECT key FROM execution_large_values ORDER BY key`) + ).map((row) => row.key) + ).toEqual(['protected', 'recent']) + }) +}) diff --git a/apps/sim/lib/cleanup/bounded.test.ts b/apps/sim/lib/cleanup/bounded.test.ts new file mode 100644 index 00000000000..140c6674f95 --- /dev/null +++ b/apps/sim/lib/cleanup/bounded.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, it, vi } from 'vitest' +import { BoundedCleanup } from '@/lib/cleanup/bounded' +import type { CleanupProgress } from '@/lib/cleanup/bounded-types' + +function setup(dryRun = false, now = () => 0) { + const snapshots: CleanupProgress[] = [] + const control = new BoundedCleanup( + { limits: { workflows: 5 }, batchSize: 2, dryRun, requestId: 'test' }, + async (progress) => { + snapshots.push(progress) + }, + now + ) + return { control, snapshots } +} + +describe('bounded cleanup accounting', () => { + it('shares one budget across owner scopes and shrinks the final batch', async () => { + const { control } = setup() + const limits: number[] = [] + for (const scope of [ + ['a', 'b', 'c'], + ['d', 'e', 'f'], + ]) { + await control.batches( + 'workflows', + async (limit, seen) => { + limits.push(limit) + return scope.filter((id) => !seen.includes(id)).slice(0, limit) + }, + (id) => id, + (rows) => control.deleted('workflows', rows.length) + ) + } + expect(control.progress.stages.workflows).toMatchObject({ selected: 5, deleted: 5 }) + expect(limits).toEqual([2, 2, 2]) + expect((await control.finish()).stopReason).toBe('budgets_exhausted') + }) + it('charges restored/skipped roots instead of refilling the budget', async () => { + const { control } = setup() + const ids = ['a', 'b', 'c', 'd', 'e', 'f'] + const limits: number[] = [] + await control.batches( + 'workflows', + async (limit, seen) => { + limits.push(limit) + return ids.filter((id) => !seen.includes(id)).slice(0, limit) + }, + (id) => id, + async () => {} + ) + expect(limits).toEqual([2, 2, 1]) + expect(control.progress.stages.workflows).toMatchObject({ selected: 5, deleted: 0, skipped: 5 }) + }) + it('dry runs enumerate distinct roots without invoking side effects', async () => { + const { control } = setup(true) + const remove = vi.fn() + await control.batches( + 'workflows', + async (limit, seen) => ['a', 'b', 'c'].filter((id) => !seen.includes(id)).slice(0, limit), + (id) => id, + remove + ) + expect(remove).not.toHaveBeenCalled() + expect(control.progress.stages.workflows).toMatchObject({ selected: 3, deleted: 0, skipped: 0 }) + expect((await control.finish()).stopReason).toBe('scopes_exhausted') + }) + it('never selects omitted types', async () => { + const { control } = setup() + const select = vi.fn() + await control.batches('chats', select, (id: string) => id, vi.fn()) + expect(select).not.toHaveBeenCalled() + }) + it('stops before another batch when its work deadline passes', async () => { + let time = 0 + const { control } = setup(false, () => time) + const select = vi.fn(async () => ['a', 'b']) + await control.batches( + 'workflows', + select, + (id) => id, + async (rows) => { + await control.deleted('workflows', rows.length) + time = 120_000 + } + ) + expect(select).toHaveBeenCalledTimes(1) + expect((await control.finish()).stopReason).toBe('time_budget') + }) + it('persists completed effects and fails without starting a later batch', async () => { + const { control, snapshots } = setup() + const select = vi.fn(async () => ['a', 'b']) + const failure = new Error('storage unavailable') + await expect( + control.batches( + 'workflows', + select, + (id) => id, + async () => { + await control.deleted('workflows', 1) + throw failure + } + ) + ).rejects.toThrow(failure) + await control.fail(failure) + expect(select).toHaveBeenCalledTimes(1) + expect(snapshots.at(-1)).toMatchObject({ + stopReason: 'failed', + stage: 'workflows', + error: 'storage unavailable', + stages: { workflows: { selected: 2, deleted: 1 } }, + }) + }) +}) diff --git a/apps/sim/lib/cleanup/bounded.ts b/apps/sim/lib/cleanup/bounded.ts new file mode 100644 index 00000000000..532256f184b --- /dev/null +++ b/apps/sim/lib/cleanup/bounded.ts @@ -0,0 +1,145 @@ +import { dbFor } from '@sim/db' +import { getErrorMessage } from '@sim/utils/errors' +import { sql } from 'drizzle-orm' +import type { + BoundedCleanupOptions, + CleanupProgress, + CleanupType, +} from '@/lib/cleanup/bounded-types' + +export type CleanupTransaction = Parameters< + Parameters['transaction']>[0] +>[0] +export type CleanupQuery = (query: (tx: CleanupTransaction) => Promise) => Promise + +/** Local settings survive transaction pooling and never change another workload's defaults. */ +export async function setCleanupTimeouts(tx: Pick): Promise { + await tx.execute(sql`SET LOCAL lock_timeout = '500ms'`) + await tx.execute(sql`SET LOCAL statement_timeout = '5s'`) +} + +/** One short DB transaction; callers do storage/backend work outside this callback. */ +export const cleanupQuery: CleanupQuery = (query) => + dbFor('cleanup').transaction(async (tx) => { + await setCleanupTimeouts(tx) + return query(tx) + }) + +export class CleanupTimeBudgetReached extends Error {} + +export class BoundedCleanup { + readonly progress: CleanupProgress + private readonly startedAt: number + constructor( + readonly options: BoundedCleanupOptions, + private readonly publish: (progress: CleanupProgress) => Promise, + private readonly now: () => number = Date.now, + readonly query: CleanupQuery = cleanupQuery + ) { + this.startedAt = now() + this.progress = { + ...options, + stages: Object.fromEntries( + Object.entries(options.limits) + .filter(([, limit]) => limit > 0) + .map(([type]) => [ + type, + { selected: 0, deleted: 0, skipped: 0, filesDeleted: 0, filesFailed: 0 }, + ]) + ), + stage: 'scopes', + durationMs: 0, + } + } + + expired(): boolean { + return this.now() - this.startedAt >= 120_000 + } + assertTimeRemaining(): void { + if (this.expired()) throw new CleanupTimeBudgetReached('Cleanup work deadline reached') + } + remaining(type: CleanupType): number { + return (this.options.limits[type] ?? 0) - (this.progress.stages[type]?.selected ?? 0) + } + stopped(): boolean { + return ( + this.expired() || + Object.keys(this.options.limits).every((type) => this.remaining(type as CleanupType) === 0) + ) + } + async checkpoint(): Promise { + this.progress.durationMs = this.now() - this.startedAt + await this.publish(structuredClone(this.progress)) + } + /** Persist actual effects immediately, even if a later side effect in this batch fails. */ + async deleted(type: CleanupType, count: number): Promise { + const stage = this.progress.stages[type] + if (!stage) throw new Error(`Unselected cleanup stage: ${type}`) + stage.deleted += count + await this.checkpoint() + } + async files(type: CleanupType, deleted: number, failed: number): Promise { + const stage = this.progress.stages[type] + if (!stage) throw new Error(`Unselected cleanup stage: ${type}`) + stage.filesDeleted += deleted + stage.filesFailed += failed + await this.checkpoint() + } + + /** + * Exclude previously selected roots so dry runs and concurrent restores cannot + * repeatedly charge the same row. Mutation batches never exceed batchSize. + */ + async batches( + type: CleanupType, + select: (limit: number, seen: string[]) => Promise, + key: (row: T) => string, + remove: (rows: T[]) => Promise + ): Promise { + const seen = new Set() + while (this.remaining(type) > 0 && !this.expired()) { + this.progress.stage = type + await this.checkpoint() + const limit = Math.min(this.options.batchSize, this.remaining(type)) + const rows = await select(limit, [...seen]) + if (rows.length > limit) throw new Error(`${type} selection exceeded its batch limit`) + if (rows.length === 0) break + const stage = (this.progress.stages[type] ??= { + selected: 0, + deleted: 0, + skipped: 0, + filesDeleted: 0, + filesFailed: 0, + }) + for (const row of rows) { + const id = key(row) + if (seen.has(id)) throw new Error(`${type} selected a root twice`) + seen.add(id) + } + stage.selected += rows.length + await this.checkpoint() + if (!this.options.dryRun) { + const before = stage.deleted + await remove(rows) + stage.skipped += rows.length - (stage.deleted - before) + } + await this.checkpoint() + if (rows.length < limit) break + } + } + + async finish(): Promise { + this.progress.stopReason = this.expired() + ? 'time_budget' + : this.stopped() + ? 'budgets_exhausted' + : 'scopes_exhausted' + await this.checkpoint() + return this.progress + } + async fail(error: unknown): Promise { + this.progress.stopReason = 'failed' + this.progress.error = getErrorMessage(error) + await this.checkpoint() + } +} diff --git a/apps/sim/lib/cleanup/chat-cleanup.ts b/apps/sim/lib/cleanup/chat-cleanup.ts index 01ca0777e2d..2ef6d06b88c 100644 --- a/apps/sim/lib/cleanup/chat-cleanup.ts +++ b/apps/sim/lib/cleanup/chat-cleanup.ts @@ -1,3 +1,9 @@ +import type { BoundedCleanup } from '@/lib/cleanup/bounded' +import { deleteBoundedStorage } from '@/lib/cleanup/bounded-storage' +import type { CleanupType } from '@/lib/cleanup/bounded-types' + +type BoundedChatCleanup = { control: BoundedCleanup; type: CleanupType } + import { dbFor } from '@sim/db' import { copilotChats, copilotMessages, workspaceFiles } from '@sim/db/schema' import { createLogger } from '@sim/logger' @@ -37,35 +43,47 @@ interface FileRef { * 1. workspaceFiles rows with chatId FK (chat-scoped contexts only) * 2. fileAttachments[].key inside each copilot_messages.content */ -export async function collectChatFiles(chatIds: string[]): Promise { +export async function collectChatFiles( + chatIds: string[], + bounded?: BoundedChatCleanup +): Promise { const files: FileRef[] = [] if (chatIds.length === 0) return files const seen = new Set() - for (const chunk of chunkArray(chatIds, CHAT_FILE_COLLECT_CHUNK_SIZE)) { - const [linkedFiles, messageRows] = await Promise.all([ - cleanupDb - .select({ - key: workspaceFiles.key, - context: workspaceFiles.context, - chatId: workspaceFiles.chatId, - }) - .from(workspaceFiles) - .where( - and( - inArray(workspaceFiles.chatId, chunk), - isNull(workspaceFiles.deletedAt), - inArray(workspaceFiles.context, [...CHAT_SCOPED_CONTEXTS]) - ) - ), - // Scan every message row for the chat (no deleted_at filter): this is a - // deletion path collecting blob keys, so attachments on any row count. - cleanupDb - .select({ content: copilotMessages.content, chatId: copilotMessages.chatId }) - .from(copilotMessages) - .where(inArray(copilotMessages.chatId, chunk)), - ]) + for (const chunk of chunkArray( + chatIds, + bounded?.control.options.batchSize ?? CHAT_FILE_COLLECT_CHUNK_SIZE + )) { + bounded?.control.assertTimeRemaining() + const selectFiles = async (executor: Pick) => + Promise.all([ + executor + .select({ + key: workspaceFiles.key, + context: workspaceFiles.context, + chatId: workspaceFiles.chatId, + }) + .from(workspaceFiles) + .where( + and( + inArray(workspaceFiles.chatId, chunk), + isNull(workspaceFiles.deletedAt), + inArray(workspaceFiles.context, [...CHAT_SCOPED_CONTEXTS]) + ) + ), + // Scan every message row for the chat (no deleted_at filter): this is a + // deletion path collecting blob keys, so attachments on any row count. + executor + .select({ content: copilotMessages.content, chatId: copilotMessages.chatId }) + .from(copilotMessages) + .where(inArray(copilotMessages.chatId, chunk)), + ]) + + const [linkedFiles, messageRows] = await (bounded + ? bounded.control.query(selectFiles) + : selectFiles(cleanupDb)) for (const f of linkedFiles) { if (f.chatId && !seen.has(f.key)) { @@ -133,10 +151,13 @@ export async function deleteStorageFiles( */ export async function cleanupCopilotBackend( chatIds: string[], - label: string + label: string, + bounded?: BoundedChatCleanup ): Promise<{ deleted: number; failed: number }> { const stats = { deleted: 0, failed: 0 } + if (bounded && chatIds.length > 0 && !env.COPILOT_API_KEY) + throw new Error('COPILOT_API_KEY is required for bounded chat cleanup') if (chatIds.length === 0 || !env.COPILOT_API_KEY) { if (!env.COPILOT_API_KEY) { logger.warn(`[${label}] COPILOT_API_KEY not set, skipping copilot backend cleanup`) @@ -144,11 +165,13 @@ export async function cleanupCopilotBackend( return stats } - for (let i = 0; i < chatIds.length; i += COPILOT_CLEANUP_BATCH_SIZE) { - const chunk = chatIds.slice(i, i + COPILOT_CLEANUP_BATCH_SIZE) + const batchSize = bounded?.control.options.batchSize ?? COPILOT_CLEANUP_BATCH_SIZE + for (let i = 0; i < chatIds.length; i += batchSize) { + const chunk = chatIds.slice(i, i + batchSize) try { const response = await fetch(`${SIM_AGENT_API_URL}/api/tasks/cleanup`, { method: 'POST', + ...(bounded ? { signal: AbortSignal.timeout(10_000) } : {}), headers: { 'Content-Type': 'application/json', 'x-api-key': env.COPILOT_API_KEY, @@ -157,6 +180,7 @@ export async function cleanupCopilotBackend( }) if (!response.ok) { + if (bounded) throw new Error(`Copilot backend cleanup failed: ${response.status}`) const errorBody = await response.text().catch(() => '') logger.error(`[${label}] Copilot backend cleanup failed: ${response.status}`, { errorBody, @@ -167,11 +191,17 @@ export async function cleanupCopilotBackend( } const result = await response.json() + if ( + bounded && + (!Number.isInteger(result.deleted) || result.deleted < 0 || (result.failed ?? 0) > 0) + ) + throw new Error('Invalid or failed Copilot backend cleanup result') stats.deleted += result.deleted ?? 0 logger.info( `[${label}] Copilot backend cleanup: ${result.deleted} chats deleted (batch ${Math.floor(i / COPILOT_CLEANUP_BATCH_SIZE) + 1})` ) } catch (error) { + if (bounded) throw error stats.failed += chunk.length logger.error(`[${label}] Copilot backend cleanup request failed:`, { error }) } @@ -191,10 +221,13 @@ export async function cleanupCopilotBackend( */ export async function prepareChatCleanup( chatIds: string[], - label: string + label: string, + bounded?: BoundedChatCleanup ): Promise<{ execute: () => Promise }> { // Collect file refs BEFORE DB deletion (keys + context are lost after cascade) - const files = await collectChatFiles(chatIds) + if (bounded && chatIds.length > 0 && !env.COPILOT_API_KEY) + throw new Error('COPILOT_API_KEY is required for bounded chat cleanup') + const files = await collectChatFiles(chatIds, bounded) if (files.length > 0) { logger.info(`[${label}] Collected ${files.length} files for cleanup`, { files: files.map((f) => ({ key: f.key, context: f.context })), @@ -207,11 +240,18 @@ export async function prepareChatCleanup( // the caller's row delete. Purge backend data and files only for chats // whose rows are actually gone, so a surviving row never loses its data. const survivors = new Set() - for (const chunk of chunkArray(chatIds, CHAT_FILE_COLLECT_CHUNK_SIZE)) { - const rows = await cleanupDb - .select({ id: copilotChats.id }) - .from(copilotChats) - .where(inArray(copilotChats.id, chunk)) + for (const chunk of chunkArray( + chatIds, + bounded?.control.options.batchSize ?? CHAT_FILE_COLLECT_CHUNK_SIZE + )) { + const selectSurvivors = async (executor: Pick) => + executor + .select({ id: copilotChats.id }) + .from(copilotChats) + .where(inArray(copilotChats.id, chunk)) + const rows = await (bounded + ? bounded.control.query(selectSurvivors) + : selectSurvivors(cleanupDb)) for (const row of rows) survivors.add(row.id) } if (survivors.size > 0) { @@ -224,7 +264,7 @@ export async function prepareChatCleanup( // Call copilot backend if (confirmedChatIds.length > 0) { - const copilotResult = await cleanupCopilotBackend(confirmedChatIds, label) + const copilotResult = await cleanupCopilotBackend(confirmedChatIds, label, bounded) logger.info( `[${label}] Copilot backend: ${copilotResult.deleted} deleted, ${copilotResult.failed} failed` ) @@ -232,6 +272,17 @@ export async function prepareChatCleanup( // Delete storage files with correct context per file if (confirmedFiles.length > 0) { + if (bounded) { + for (const context of CHAT_SCOPED_CONTEXTS) { + await deleteBoundedStorage( + bounded.control, + bounded.type, + confirmedFiles.filter((file) => file.context === context).map((file) => file.key), + context + ) + } + return + } const fileStats = await deleteStorageFiles(confirmedFiles, label) logger.info( `[${label}] Storage cleanup: ${fileStats.filesDeleted} deleted, ${fileStats.filesFailed} failed` diff --git a/apps/sim/lib/cleanup/queue.ts b/apps/sim/lib/cleanup/queue.ts new file mode 100644 index 00000000000..64a5bf8945d --- /dev/null +++ b/apps/sim/lib/cleanup/queue.ts @@ -0,0 +1,2 @@ +/** Logs and soft deletes share one lane, including legacy scheduled runs. */ +export const retentionCleanupQueue = { name: 'retention-cleanup', concurrencyLimit: 1 } diff --git a/apps/sim/lib/cleanup/vitest.postgres.config.ts b/apps/sim/lib/cleanup/vitest.postgres.config.ts new file mode 100644 index 00000000000..e17b3a9df68 --- /dev/null +++ b/apps/sim/lib/cleanup/vitest.postgres.config.ts @@ -0,0 +1,13 @@ +import path from 'node:path' +import { defineConfig } from 'vitest/config' + +/** Isolated real-Postgres suite; deliberately does not load app env files or global DB mocks. */ +export default defineConfig({ + test: { + include: ['lib/cleanup/bounded.postgres.integration.ts'], + environment: 'node', + fileParallelism: false, + testTimeout: 15_000, + }, + resolve: { tsconfigPaths: true, alias: { '@': path.resolve(import.meta.dirname, '../..') } }, +}) diff --git a/apps/sim/lib/execution/payloads/large-value-metadata.ts b/apps/sim/lib/execution/payloads/large-value-metadata.ts index dcbc03f4ff6..7a188fef613 100644 --- a/apps/sim/lib/execution/payloads/large-value-metadata.ts +++ b/apps/sim/lib/execution/payloads/large-value-metadata.ts @@ -372,26 +372,7 @@ async function pruneStaleReferences( SELECT ref.ctid FROM ${executionLargeValueReferences} AS ref WHERE ref.workspace_id IN ${workspaceIds} - AND ( - ( - ref.source = 'execution_log' - AND NOT EXISTS ( - SELECT 1 - FROM ${workflowExecutionLogs} AS wel - WHERE wel.execution_id = ref.execution_id - ) - ) - OR ( - ref.source = 'paused_snapshot' - AND NOT EXISTS ( - SELECT 1 - FROM ${pausedExecutions} AS pe - WHERE pe.execution_id = ref.execution_id - AND pe.status IN ${LIVE_PAUSED_REFERENCE_STATUSES} - ) - ) - OR ref.source NOT IN ('execution_log', 'paused_snapshot') - ) + AND ${staleLargeValueReferencePredicate()} LIMIT ${batchSize} ) RETURNING ref.key @@ -416,19 +397,7 @@ async function pruneDeletedParentDependencies( SELECT dependency.ctid FROM ${executionLargeValueDependencies} AS dependency WHERE dependency.workspace_id IN ${workspaceIds} - AND ( - EXISTS ( - SELECT 1 - FROM ${executionLargeValues} AS parent_value - WHERE parent_value.key = dependency.parent_key - AND parent_value.deleted_at IS NOT NULL - ) - OR NOT EXISTS ( - SELECT 1 - FROM ${executionLargeValues} AS parent_value - WHERE parent_value.key = dependency.parent_key - ) - ) + AND ${staleLargeValueDependencyPredicate()} LIMIT ${batchSize} ) RETURNING dependency.parent_key @@ -454,13 +423,7 @@ async function pruneDeletedLargeValueTombstones( SELECT value.ctid FROM ${executionLargeValues} AS value WHERE value.workspace_id IN ${workspaceIds} - AND value.deleted_at IS NOT NULL - AND value.deleted_at < ${sql.param(deletedBefore, executionLargeValues.deletedAt)} - AND NOT EXISTS ( - SELECT 1 - FROM ${executionLargeValueDependencies} AS dependency - WHERE dependency.parent_key = value.key - ) + AND ${largeValueTombstonePredicate(deletedBefore)} LIMIT ${batchSize} ) RETURNING value.key @@ -613,3 +576,55 @@ export function unreferencedLargeValuePredicate() { ) ` } + +/** Eligibility shared by bounded maintenance and scheduled pruning. SQL alias: ref. */ +export function staleLargeValueReferencePredicate() { + return sql`( + ( + ref.source = 'execution_log' + AND NOT EXISTS ( + SELECT 1 + FROM ${workflowExecutionLogs} AS wel + WHERE wel.execution_id = ref.execution_id + ) + ) + OR ( + ref.source = 'paused_snapshot' + AND NOT EXISTS ( + SELECT 1 + FROM ${pausedExecutions} AS pe + WHERE pe.execution_id = ref.execution_id + AND pe.status IN ${LIVE_PAUSED_REFERENCE_STATUSES} + ) + ) + OR ref.source NOT IN ('execution_log', 'paused_snapshot') + )` +} + +/** Eligibility shared by bounded maintenance and scheduled pruning. SQL alias: dependency. */ +export function staleLargeValueDependencyPredicate() { + return sql`( + EXISTS ( + SELECT 1 + FROM ${executionLargeValues} AS parent_value + WHERE parent_value.key = dependency.parent_key + AND parent_value.deleted_at IS NOT NULL + ) + OR NOT EXISTS ( + SELECT 1 + FROM ${executionLargeValues} AS parent_value + WHERE parent_value.key = dependency.parent_key + ) + )` +} + +/** Eligibility shared by bounded maintenance and scheduled pruning. SQL alias: value. */ +export function largeValueTombstonePredicate(deletedBefore: Date) { + return sql`value.deleted_at IS NOT NULL + AND value.deleted_at < ${sql.param(deletedBefore, executionLargeValues.deletedAt)} + AND NOT EXISTS ( + SELECT 1 + FROM ${executionLargeValueDependencies} AS dependency + WHERE dependency.parent_key = value.key + )` +} diff --git a/apps/sim/lib/knowledge/documents/service.ts b/apps/sim/lib/knowledge/documents/service.ts index 188a5c0308d..4eaf79588f6 100644 --- a/apps/sim/lib/knowledge/documents/service.ts +++ b/apps/sim/lib/knowledge/documents/service.ts @@ -49,6 +49,7 @@ import { } from '@/lib/billing/storage' import { checkAndBillPayerOverageThreshold } from '@/lib/billing/threshold-billing' import type { ChunkingStrategy, StrategyOptions } from '@/lib/chunkers/types' +import type { CleanupQuery } from '@/lib/cleanup/bounded' import { resolveTriggerRegion } from '@/lib/core/async-jobs/region' import { env, envNumber } from '@/lib/core/config/env' import { getCostMultiplier, isTriggerDevEnabled } from '@/lib/core/config/env-flags' @@ -3948,7 +3949,8 @@ export async function hardDeleteDocuments( expectedKnowledgeBaseId?: string, connectorSyncGuard?: ConnectorSyncDeletionGuard, /** When provided, only documents the caller may currently read are deleted, re-verified at the delete itself. */ - access?: KnowledgeAccessScope + access?: KnowledgeAccessScope, + cleanupQuery?: CleanupQuery ): Promise { const ids = [...new Set(documentIds)] if (ids.length === 0) { @@ -3963,7 +3965,8 @@ export async function hardDeleteDocuments( expectedConnectorId, expectedKnowledgeBaseId, connectorSyncGuard, - access + access, + cleanupQuery ) } return deletedCount @@ -3979,7 +3982,8 @@ async function hardDeleteDocumentBatch( expectedConnectorId?: string, expectedKnowledgeBaseId?: string, connectorSyncGuard?: ConnectorSyncDeletionGuard, - access?: KnowledgeAccessScope + access?: KnowledgeAccessScope, + cleanupQuery?: CleanupQuery ): Promise { const ids = [...new Set(documentIds)] const scopedConnectorId = connectorSyncGuard?.connectorId ?? expectedConnectorId @@ -3987,31 +3991,36 @@ async function hardDeleteDocumentBatch( const requireEligibleDocument = Boolean(expectedKnowledgeBaseId || connectorSyncGuard) const requireVisibleDocument = Boolean(expectedKnowledgeBaseId && !connectorSyncGuard) const accessCondition = access ? knowledgeAccessCondition(access) : undefined - const documentsToDelete = await db - .select({ - id: document.id, - knowledgeBaseId: document.knowledgeBaseId, - fileUrl: document.fileUrl, - fileSize: document.fileSize, - uploadedBy: document.uploadedBy, - connectorId: document.connectorId, - deletedAt: document.deletedAt, - workspaceId: knowledgeBase.workspaceId, - organizationId: knowledgeBase.organizationId, - }) - .from(document) - .innerJoin(knowledgeBase, eq(document.knowledgeBaseId, knowledgeBase.id)) - .where( - and( - inArray(document.id, ids), - scopedConnectorId ? eq(document.connectorId, scopedConnectorId) : undefined, - scopedKnowledgeBaseId ? eq(document.knowledgeBaseId, scopedKnowledgeBaseId) : undefined, - requireEligibleDocument ? eq(document.userExcluded, false) : undefined, - requireEligibleDocument ? isNull(document.archivedAt) : undefined, - requireVisibleDocument ? isNull(document.deletedAt) : undefined, - accessCondition + const selectDocuments = async (executor: Pick) => + executor + .select({ + id: document.id, + knowledgeBaseId: document.knowledgeBaseId, + fileUrl: document.fileUrl, + fileSize: document.fileSize, + uploadedBy: document.uploadedBy, + connectorId: document.connectorId, + deletedAt: document.deletedAt, + workspaceId: knowledgeBase.workspaceId, + organizationId: knowledgeBase.organizationId, + }) + .from(document) + .innerJoin(knowledgeBase, eq(document.knowledgeBaseId, knowledgeBase.id)) + .where( + and( + inArray(document.id, ids), + scopedConnectorId ? eq(document.connectorId, scopedConnectorId) : undefined, + scopedKnowledgeBaseId ? eq(document.knowledgeBaseId, scopedKnowledgeBaseId) : undefined, + requireEligibleDocument ? eq(document.userExcluded, false) : undefined, + requireEligibleDocument ? isNull(document.archivedAt) : undefined, + requireVisibleDocument ? isNull(document.deletedAt) : undefined, + accessCondition + ) ) - ) + + const documentsToDelete = await (cleanupQuery + ? cleanupQuery(selectDocuments) + : selectDocuments(db)) if (documentsToDelete.length === 0) { return 0 @@ -4031,7 +4040,11 @@ async function hardDeleteDocumentBatch( if (!storageContextByWorkspace.has(doc.workspaceId)) { storageContextByWorkspace.set( doc.workspaceId, - await resolveStorageBillingContext(doc.workspaceId) + await (cleanupQuery + ? cleanupQuery((executor) => + resolveStorageBillingContext(doc.workspaceId!, { executor }) + ) + : resolveStorageBillingContext(doc.workspaceId)) ) } continue @@ -4044,7 +4057,7 @@ async function hardDeleteDocumentBatch( * concurrent deletion cannot double-decrement a payer. */ let deletedDocs: typeof documentsToDelete = [] - await db.transaction(async (tx) => { + const deleteBatch = async (tx: Parameters[0]>[0]) => { /** * Lock every parent KB in stable ID order before deleting document rows. * Normal inserts and KB moves take the same parent lock first, so the @@ -4188,7 +4201,8 @@ async function hardDeleteDocumentBatch( }), legacyDeltas: [], }) - }) + } + await (cleanupQuery ? cleanupQuery(deleteBatch) : db.transaction(deleteBatch)) logger.info(`[${requestId}] Hard deleted ${deletedDocs.length} documents`, { documentIds: deletedDocs.map((doc) => doc.id), diff --git a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts index 33221719dea..7de03975a7d 100644 --- a/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts +++ b/apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts @@ -383,14 +383,15 @@ function withCopySuffix(fileName: string, n: number): string { export async function allocateUniqueWorkspaceFileName( workspaceId: string, baseName: string, - folderId?: string | null + folderId?: string | null, + exists: typeof fileExistsInWorkspace = fileExistsInWorkspace ): Promise { - if (!(await fileExistsInWorkspace(workspaceId, baseName, folderId))) { + if (!(await exists(workspaceId, baseName, folderId))) { return baseName } for (let n = 1; n <= MAX_COPY_SUFFIX; n++) { const candidate = withCopySuffix(baseName, n) - if (!(await fileExistsInWorkspace(workspaceId, candidate, folderId))) { + if (!(await exists(workspaceId, candidate, folderId))) { return candidate } } From af6598d8418abe97c39aa334c64d9d25d15bf8a2 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Mon, 14 Sep 2026 19:29:03 -0700 Subject: [PATCH 2/4] fix(cleanup): coordinate deletion with resource eligibility --- apps/sim/background/cleanup-bounded.test.ts | 44 +++++- apps/sim/background/cleanup-logs-bounded.ts | 47 +++--- .../cleanup-soft-deletes-bounded.ts | 108 ++++++++------ .../executor/execution/block-executor.test.ts | 5 + .../variables/resolvers/workflow.test.ts | 5 + apps/sim/lib/cleanup/bounded-cleanup.md | 6 +- apps/sim/lib/cleanup/bounded-delete.ts | 32 +++- .../cleanup/bounded.postgres.integration.ts | 89 +++++++++++- .../payloads/large-array-manifest.test.ts | 5 + .../execution/payloads/large-value-lock.ts | 44 ++++++ .../payloads/large-value-metadata.test.ts | 18 +-- .../payloads/large-value-metadata.ts | 137 +++++++++--------- .../payloads/prune-metadata-sql.test.ts | 7 + .../knowledge/documents/storage-cleanup.ts | 43 ++++-- 14 files changed, 413 insertions(+), 177 deletions(-) create mode 100644 apps/sim/lib/execution/payloads/large-value-lock.ts diff --git a/apps/sim/background/cleanup-bounded.test.ts b/apps/sim/background/cleanup-bounded.test.ts index 86651637c8f..3e6deeff1f2 100644 --- a/apps/sim/background/cleanup-bounded.test.ts +++ b/apps/sim/background/cleanup-bounded.test.ts @@ -120,18 +120,56 @@ describe('requested cleanup stages', () => { expect(run.progress.stages.jobLogs).toMatchObject({ selected: 2, deleted: 2 }) expect(dbChainMockFns.delete).toHaveBeenCalledTimes(2) }) - it('stops before root deletion if attached log storage fails', async () => { + it('records committed log deletion if attached storage cleanup fails', async () => { queueTableRows(schemaMock.workflowExecutionLogs, [{ id: 'one', files: [{ key: 'blob' }] }]) + dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'one', files: [{ key: 'blob' }] }]) storage.mockResolvedValue({ deleted: 0, failed: [{ key: 'blob', error: 'unavailable' }] }) const run = control('workflowLogs', false) await expect(runBoundedLogScope(scope, run)).rejects.toThrow('storage deletions failed') - expect(dbChainMockFns.delete).not.toHaveBeenCalled() + expect(dbChainMockFns.delete).toHaveBeenCalledOnce() expect(run.progress.stages.workflowLogs).toMatchObject({ selected: 1, - deleted: 0, + deleted: 1, filesFailed: 1, }) }) + it('does not remove files of a log protected after selection', async () => { + queueTableRows(schemaMock.workflowExecutionLogs, [{ id: 'one', files: [{ key: 'blob' }] }]) + dbChainMockFns.returning.mockResolvedValueOnce([]) + const run = control('workflowLogs', false) + await runBoundedLogScope(scope, run) + expect(storage).not.toHaveBeenCalled() + expect(dbChainMockFns.update).not.toHaveBeenCalled() + expect(run.progress.stages.workflowLogs).toMatchObject({ selected: 1, deleted: 0, skipped: 1 }) + }) + it.each(['largeValues', 'legacyLargeValues'] as const)( + 'does not remove a %s key that fails the final liveness claim', + async (type) => { + queueTableRows( + type === 'largeValues' ? schemaMock.executionLargeValues : schemaMock.workspaceFiles, + [{ key: 'referenced-key' }] + ) + dbChainMockFns.returning.mockResolvedValueOnce([]) + const run = control(type, false) + await runBoundedLogScope(scope, run) + expect(storage).not.toHaveBeenCalled() + expect(run.progress.stages[type]).toMatchObject({ selected: 1, deleted: 0, skipped: 1 }) + } + ) + it.each(['files', 'legacyFiles'] as const)( + 'does not remove a restored %s object', + async (type) => { + queueTableRows(type === 'files' ? schemaMock.workspaceFiles : schemaMock.workspaceFile, [ + { id: 'one', key: 'blob', context: 'workspace', workspaceId: 'ws-one', sizeBytes: 100 }, + ]) + billing.mockResolvedValue({ workspaceId: 'ws-one' }) + dbChainMockFns.returning.mockResolvedValueOnce([]) + const run = control(type, false) + await runBoundedSoftDeleteScope(scope, run) + expect(storage).not.toHaveBeenCalled() + expect(run.progress.stages[type]).toMatchObject({ selected: 1, deleted: 0, skipped: 1 }) + } + ) it('keeps workflow chat side effects even with a zero chat budget', async () => { queueTableRows(schemaMock.workflow, [{ id: 'workflow-one' }]) queueTableRows(schemaMock.copilotChats, [{ id: 'child-chat' }]) diff --git a/apps/sim/background/cleanup-logs-bounded.ts b/apps/sim/background/cleanup-logs-bounded.ts index 6a3bc9264bc..40d14698094 100644 --- a/apps/sim/background/cleanup-logs-bounded.ts +++ b/apps/sim/background/cleanup-logs-bounded.ts @@ -51,17 +51,6 @@ export async function runBoundedLogScope(payload: CleanupJobPayload, control: Bo ), (row) => row.id, async (rows) => { - for (const row of rows) { - const keys = Array.isArray(row.files) - ? row.files.flatMap((file) => - file && typeof file === 'object' && 'key' in file && typeof file.key === 'string' - ? [file.key] - : [] - ) - : [] - await deleteBoundedStorage(control, 'workflowLogs', keys, 'execution') - if (isUsingCloudStorage()) await tombstoneBoundedFiles(control, keys) - } const deleted = await control.query(async (tx) => tx .delete(workflowExecutionLogs) @@ -74,9 +63,20 @@ export async function runBoundedLogScope(payload: CleanupJobPayload, control: Bo ) ) ) - .returning({ id: workflowExecutionLogs.id }) + .returning({ id: workflowExecutionLogs.id, files: workflowExecutionLogs.files }) ) await control.deleted('workflowLogs', deleted.length) + for (const row of deleted) { + const keys = Array.isArray(row.files) + ? row.files.flatMap((file) => + file && typeof file === 'object' && 'key' in file && typeof file.key === 'string' + ? [file.key] + : [] + ) + : [] + await deleteBoundedStorage(control, 'workflowLogs', keys, 'execution') + if (isUsingCloudStorage()) await tombstoneBoundedFiles(control, keys) + } } ) await boundedDelete( @@ -122,15 +122,22 @@ export async function runBoundedLogScope(payload: CleanupJobPayload, control: Bo (row) => row.key, async (rows) => { if (!isUsingCloudStorage()) return - const keys = rows.map((row) => row.key) + const selectedKeys = rows.map((row) => row.key) + const keys = await control.query(async (tx) => { + await tx + .select({ key: table.key }) + .from(table) + .where(inArray(table.key, selectedKeys)) + .orderBy(asc(table.key)) + .for('update') + const claimed = await tx + .update(table) + .set({ deletedAt: new Date() }) + .where(and(eligible, inArray(table.key, selectedKeys))) + .returning({ key: table.key }) + return claimed.map((row) => row.key) + }) await deleteBoundedStorage(control, type, keys, 'execution') - if (!legacy) - await control.query(async (tx) => { - await tx - .update(executionLargeValues) - .set({ deletedAt: new Date() }) - .where(inArray(executionLargeValues.key, keys)) - }) await control.deleted(type, keys.length) await tombstoneBoundedFiles(control, keys) } diff --git a/apps/sim/background/cleanup-soft-deletes-bounded.ts b/apps/sim/background/cleanup-soft-deletes-bounded.ts index 4196a9dbac7..bc63b8187c2 100644 --- a/apps/sim/background/cleanup-soft-deletes-bounded.ts +++ b/apps/sim/background/cleanup-soft-deletes-bounded.ts @@ -30,6 +30,7 @@ import { resolveCleanupOwnerScope, } from '@/lib/cleanup/resource-scope' import { hardDeleteDocuments } from '@/lib/knowledge/documents/service' +import { cleanupKnowledgeStorageBinding } from '@/lib/knowledge/documents/storage-cleanup' import type { StorageContext } from '@/lib/uploads' import { getWorkspaceFileSize } from '@/lib/uploads/shared/types' import { reRootActiveFolderChildrenUnguarded } from '@/background/cleanup-soft-deletes' @@ -121,18 +122,16 @@ export async function runBoundedSoftDeleteScope( lt(knowledgeBase.deletedAt, cutoff) ), { - before: async (kbIds) => { + beforeDelete: async (kbIds, tx) => { // Existing ledger/outbox implementation owns document and embedding deletion. while (true) { control.assertTimeRemaining() - const rows = await control.query(async (tx) => - tx - .select({ id: document.id }) - .from(document) - .where(inArray(document.knowledgeBaseId, kbIds)) - .orderBy(asc(document.id)) - .limit(control.options.batchSize) - ) + const rows = await tx + .select({ id: document.id }) + .from(document) + .where(inArray(document.knowledgeBaseId, kbIds)) + .orderBy(asc(document.id)) + .limit(control.options.batchSize) if (rows.length === 0) break const deleted = await hardDeleteDocuments( rows.map((row) => row.id), @@ -141,7 +140,7 @@ export async function runBoundedSoftDeleteScope( undefined, undefined, undefined, - control.query + async (query) => query(tx) ) if (deleted !== rows.length) throw new Error('Knowledge-base document cleanup did not delete its selected batch') @@ -176,10 +175,8 @@ export async function runBoundedSoftDeleteScope( ), target.type === 'folders' ? { - before: (folderIds) => - control.query((tx) => - reRootActiveFolderChildrenUnguarded(folderIds, cutoff, payload.label, tx, true) - ), + beforeDelete: (folderIds, tx) => + reRootActiveFolderChildrenUnguarded(folderIds, cutoff, payload.label, tx, true), } : {} ) @@ -210,12 +207,6 @@ async function cleanupFiles(control: BoundedCleanup, scope: CleanupOwnerScope, c ), (row) => row.id, async (rows) => { - await deleteBoundedStorage( - control, - 'legacyFiles', - rows.map((row) => row.key), - 'workspace' - ) const deleted = await control.query(async (tx) => tx .delete(workspaceFile) @@ -228,9 +219,15 @@ async function cleanupFiles(control: BoundedCleanup, scope: CleanupOwnerScope, c ) ) ) - .returning({ id: workspaceFile.id }) + .returning({ id: workspaceFile.id, key: workspaceFile.key }) ) await control.deleted('legacyFiles', deleted.length) + await deleteBoundedStorage( + control, + 'legacyFiles', + deleted.map((row) => row.key), + 'workspace' + ) } ) } @@ -270,7 +267,6 @@ async function cleanupFiles(control: BoundedCleanup, scope: CleanupOwnerScope, c return resolveStorageBillingContext(row.workspaceId, { executor }) }) : undefined - await deleteBoundedStorage(control, 'files', [row.key], row.context as StorageContext) const remove = async (tx: Parameters[0]>[0]) => { const deleted = await tx .delete(workspaceFiles) @@ -282,14 +278,18 @@ async function cleanupFiles(control: BoundedCleanup, scope: CleanupOwnerScope, c billing ? eq(workspaceFiles.workspaceId, billing.workspaceId) : undefined ) ) - .returning({ id: workspaceFiles.id, sizeBytes: workspaceFiles.sizeBytes }) + .returning({ + id: workspaceFiles.id, + key: workspaceFiles.key, + sizeBytes: workspaceFiles.sizeBytes, + }) if (billing) await decrementStorageUsageForBillingContextInTx( tx, billing, deleted.reduce((sum, file) => sum + getWorkspaceFileSize(file), 0) ) - return deleted.length + return deleted } const deleted = billing ? await db.transaction(async (tx) => { @@ -297,7 +297,13 @@ async function cleanupFiles(control: BoundedCleanup, scope: CleanupOwnerScope, c return remove(tx) }) : await control.query(remove) - await control.deleted('files', deleted) + await control.deleted('files', deleted.length) + await deleteBoundedStorage( + control, + 'files', + deleted.map((file) => file.key), + row.context as StorageContext + ) } } ) @@ -317,7 +323,14 @@ async function cleanupOrphanBindings(control: BoundedCleanup, scope: CleanupOwne (limit, seen) => control.query(async (tx) => tx - .select({ id: workspaceFiles.id, key: workspaceFiles.key }) + .select({ + id: workspaceFiles.id, + key: workspaceFiles.key, + contentUpdatedAt: workspaceFiles.contentUpdatedAt, + workspaceId: workspaceFiles.workspaceId, + organizationId: workspaceFiles.organizationId, + userId: workspaceFiles.userId, + }) .from(workspaceFiles) .where(and(eligible, seen.length ? notInArray(workspaceFiles.id, seen) : undefined)) .orderBy(asc(workspaceFiles.id)) @@ -325,28 +338,27 @@ async function cleanupOrphanBindings(control: BoundedCleanup, scope: CleanupOwne ), (row) => row.id, async (rows) => { - await deleteBoundedStorage( - control, - type, - rows.map((row) => row.key), - 'knowledge-base' - ) - const deleted = await control.query(async (tx) => - tx - .update(workspaceFiles) - .set({ deletedAt: new Date() }) - .where( - and( - eligible, - inArray( - workspaceFiles.id, - rows.map((row) => row.id) - ) - ) - ) - .returning({ id: workspaceFiles.id }) - ) - await control.deleted(type, deleted.length) + for (const row of rows) { + control.assertTimeRemaining() + const deleted = await cleanupKnowledgeStorageBinding( + { + version: 1, + documentId: `orphan:${row.id}`, + fileId: row.id, + key: row.key, + contentUpdatedAt: row.contentUpdatedAt.toISOString(), + workspaceId: row.workspaceId, + organizationId: row.organizationId, + userId: row.userId, + }, + AbortSignal.timeout(15_000), + control.query + ) + if (deleted) { + await control.deleted(type, 1) + await control.files(type, 1, 0) + } + } } ) } diff --git a/apps/sim/executor/execution/block-executor.test.ts b/apps/sim/executor/execution/block-executor.test.ts index 1cae20df636..af542fdc222 100644 --- a/apps/sim/executor/execution/block-executor.test.ts +++ b/apps/sim/executor/execution/block-executor.test.ts @@ -31,6 +31,11 @@ const { mockUploadFile, mockDownloadFile, mockMaskBatch } = vi.hoisted(() => ({ mockMaskBatch: vi.fn(), })) +vi.mock('@/lib/execution/payloads/large-value-metadata', () => ({ + registerLargeValueOwner: vi.fn().mockResolvedValue(true), + addLargeValueReference: vi.fn().mockResolvedValue(undefined), +})) + vi.mock('@/ee/access-control/utils/permission-check', () => ({ validateBlockType: vi.fn(), })) diff --git a/apps/sim/executor/variables/resolvers/workflow.test.ts b/apps/sim/executor/variables/resolvers/workflow.test.ts index 0055aaed708..858e6c41b31 100644 --- a/apps/sim/executor/variables/resolvers/workflow.test.ts +++ b/apps/sim/executor/variables/resolvers/workflow.test.ts @@ -9,6 +9,11 @@ import { navigatePathAsync } from '@/executor/variables/resolvers/reference-asyn import type { ResolutionContext } from './reference' import { WorkflowResolver } from './workflow' +vi.mock('@/lib/execution/payloads/large-value-metadata', () => ({ + registerLargeValueOwner: vi.fn().mockResolvedValue(true), + addLargeValueReference: vi.fn().mockResolvedValue(undefined), +})) + vi.mock('@/lib/workflows/variables/variable-manager', () => ({ VariableManager: { resolveForExecution: vi.fn((value) => value), diff --git a/apps/sim/lib/cleanup/bounded-cleanup.md b/apps/sim/lib/cleanup/bounded-cleanup.md index 78963fb17f1..ec7ca54671f 100644 --- a/apps/sim/lib/cleanup/bounded-cleanup.md +++ b/apps/sim/lib/cleanup/bounded-cleanup.md @@ -42,7 +42,7 @@ A call with **no query parameters retains the existing scheduled cleanup behavio ## What the budget means -Limits count **selected roots**, including roots restored before deletion. They do not count all physical rows affected by foreign keys. Two workflows can cascade into thousands of blocks, edges, chats, and messages. Mandatory attached-file and backend cleanup follows selected parents even when its standalone type budget is zero. Knowledge-base deletion retains the existing document accounting and storage-cleanup outbox behavior. +Limits count **selected roots**, including roots restored before deletion. They do not count all physical rows affected by foreign keys. Two workflows can cascade into thousands of blocks, edges, chats, and messages. Mandatory attached-file and backend cleanup follows selected parents even when its standalone type budget is zero. Knowledge-base deletion retains the existing document accounting and storage-cleanup outbox behavior. Its child changes and parent deletion share one locked transaction, as do folder re-rooting and deletion; a failure rolls all of them back. `largeValues`/`legacyLargeValues` count object keys. `orphanKnowledgeBaseBindings` counts bindings soft-deleted after object cleanup. Metadata pruning counts its selected metadata records. A dry run previews the current state; it does not reserve rows for a later deletion run. @@ -51,9 +51,9 @@ Limits count **selected roots**, including roots restored before deletion. They - One coordinator walks owner scopes sequentially with shared budgets. It creates no child cleanup jobs. - Logs and soft deletes share the named Trigger queue `retention-cleanup`, concurrency 1, including newly dispatched scheduled jobs. No per-type concurrency keys are used. - Bounded dispatch sets one attempt and a 180-second hard maximum. The worker stops starting new root batches after 120 seconds; cancellable child preparation also observes that work deadline. -- Cleanup SQL uses transaction-local **500ms lock_timeout** and **5s statement_timeout**. The billable-file delete and storage decrement remain atomic. +- Cleanup SQL uses transaction-local **500ms lock_timeout** and **5s statement_timeout**. The billable-file delete and storage decrement remain atomic. Orphan knowledge-base storage cleanup reuses the binding lock held by document creation, with a 15-second storage deadline. Large-value reference writers lock the value before registering references; cleanup locks and rechecks it before claiming a tombstone. - Trigger `cleanup` metadata reports each requested type's `selected`, `deleted`, `skipped`, `filesDeleted`, and `filesFailed`, plus stage, duration, and stop reason: `budgets_exhausted`, `scopes_exhausted`, `time_budget`, or `failed`. -- A failure stops subsequent stages, preserves completed progress, and fails the run. External deletion is not transactional with Postgres. A hard process termination may leave the last metadata checkpoint behind actual effects. Do not blindly replay failed runs: inspect the failed stage and any external work already completed. This change does not add recovery for the existing gap when chat backend/storage cleanup fails after parent rows have committed. +- A failure stops subsequent stages, preserves completed progress, and fails the run. External deletion is not transactional with Postgres. A hard process termination may leave the last metadata checkpoint behind actual effects. Do not blindly replay failed runs: inspect the failed stage and any external work already completed. Log and file storage is removed only for rows returned by the guarded delete. A storage failure after that commit can leave orphaned objects; a failed large-value storage deletion can leave a tombstone with remaining bytes. This change does not add recovery for those gaps or for chat backend/storage cleanup failures after parent deletion. Inspect failed runs before resubmitting. ## Rollout diff --git a/apps/sim/lib/cleanup/bounded-delete.ts b/apps/sim/lib/cleanup/bounded-delete.ts index 51af402f223..72f6477a848 100644 --- a/apps/sim/lib/cleanup/bounded-delete.ts +++ b/apps/sim/lib/cleanup/bounded-delete.ts @@ -1,6 +1,6 @@ import { and, inArray, notInArray, type SQL } from 'drizzle-orm' -import type { PgColumn, PgTable } from 'drizzle-orm/pg-core' -import type { BoundedCleanup } from '@/lib/cleanup/bounded' +import type { AnyPgColumn, PgTable } from 'drizzle-orm/pg-core' +import type { BoundedCleanup, CleanupTransaction } from '@/lib/cleanup/bounded' import type { CleanupType } from '@/lib/cleanup/bounded-types' /** Select and delete the same roots, reasserting eligibility after side effects. */ @@ -8,10 +8,11 @@ export async function boundedDelete( control: BoundedCleanup, type: CleanupType, table: PgTable, - id: PgColumn, + id: AnyPgColumn<{ data: string; notNull: true }>, eligibility: SQL | undefined, hooks: { before?: (ids: string[]) => Promise + beforeDelete?: (ids: string[], tx: CleanupTransaction) => Promise after?: (ids: string[]) => Promise } = {} ) { @@ -30,14 +31,29 @@ export async function boundedDelete( async (rows) => { const ids = rows.map((row) => row.id) await hooks.before?.(ids) - const deleted = await control.query(async (tx) => - tx + const deleted = await control.query(async (tx) => { + const targeted = hooks.beforeDelete + ? await tx + .select({ id }) + .from(table) + .where(and(inArray(id, ids), eligibility)) + .for('update') + : rows + const targetedIds = targeted.map((row) => row.id) + if (targetedIds.length === 0) return [] + control.assertTimeRemaining() + await hooks.beforeDelete?.(targetedIds, tx) + control.assertTimeRemaining() + const deleted = await tx .delete(table) - .where(and(inArray(id, ids), eligibility)) + .where(and(inArray(id, targetedIds), eligibility)) .returning({ id }) - ) + if (hooks.beforeDelete && deleted.length !== targetedIds.length) + throw new Error(`${type} eligibility changed during child cleanup`) + return deleted + }) await control.deleted(type, deleted.length) - await hooks.after?.(ids) + await hooks.after?.(deleted.map((row) => row.id)) } ) } diff --git a/apps/sim/lib/cleanup/bounded.postgres.integration.ts b/apps/sim/lib/cleanup/bounded.postgres.integration.ts index f9bc37391d5..b958ccd5bd4 100644 --- a/apps/sim/lib/cleanup/bounded.postgres.integration.ts +++ b/apps/sim/lib/cleanup/bounded.postgres.integration.ts @@ -1,10 +1,11 @@ -import { db, dbFor } from '@sim/db' +import { db, dbFor, runOutsideTransactionContext } from '@sim/db' import { eq, sql } from 'drizzle-orm' import { pgTable, text } from 'drizzle-orm/pg-core' import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest' import { BoundedCleanup, cleanupQuery } from '@/lib/cleanup/bounded' import { boundedDelete } from '@/lib/cleanup/bounded-delete' import { pruneBoundedLargeValueMetadata } from '@/lib/cleanup/bounded-large-value-metadata' +import { lockLargeValueKeysForReference } from '@/lib/execution/payloads/large-value-lock' const url = new URL(process.env.DATABASE_URL ?? '') if (url.hostname !== '127.0.0.1' || url.pathname !== '/bounded_cleanup_test') { @@ -26,6 +27,9 @@ beforeAll(async () => { await db.execute( sql`CREATE TABLE execution_large_values (key text PRIMARY KEY, workspace_id text, deleted_at timestamp)` ) + await db.execute( + sql`CREATE TABLE workspace_files (key text PRIMARY KEY, context text, deleted_at timestamp)` + ) await db.execute(sql`CREATE TABLE workflow_execution_logs (execution_id text)`) await db.execute(sql`CREATE TABLE paused_executions (execution_id text, status text)`) @@ -38,7 +42,7 @@ beforeAll(async () => { }) beforeEach(async () => { await db.execute( - sql`TRUNCATE execution_large_value_references, execution_large_value_dependencies, execution_large_values, workflow_execution_logs, paused_executions` + sql`TRUNCATE execution_large_value_references, execution_large_value_dependencies, execution_large_values, workflow_execution_logs, paused_executions, workspace_files` ) await db.execute(sql`TRUNCATE cleanup_fixture_roots, cleanup_fixture_children`) @@ -53,7 +57,7 @@ beforeEach(async () => { }) afterAll(async () => { await db.execute( - sql`DROP TABLE IF EXISTS execution_large_value_references, execution_large_value_dependencies, execution_large_values, workflow_execution_logs, paused_executions` + sql`DROP TABLE IF EXISTS execution_large_value_references, execution_large_value_dependencies, execution_large_values, workflow_execution_logs, paused_executions, workspace_files` ) await db.execute(sql`DROP TABLE IF EXISTS cleanup_fixture_children, cleanup_fixture_roots`) @@ -105,6 +109,48 @@ describe('bounded cleanup against PostgreSQL', () => { expect(run.progress.stages.workflows).toMatchObject({ selected: 1, deleted: 0, skipped: 1 }) expect(await count('roots')).toBe(6) }) + it('rolls back child mutations when parent cleanup fails', async () => { + const run = control(1) + await expect( + boundedDelete(run, 'workflows', roots, roots.id, eq(roots.id, 'a'), { + beforeDelete: async (ids, tx) => { + await tx.execute(sql`DELETE FROM cleanup_fixture_children WHERE root_id = ${ids[0]}`) + throw new Error('child cleanup failed') + }, + }) + ).rejects.toThrow('child cleanup failed') + expect(await count('roots')).toBe(6) + expect(await count('children')).toBe(120) + expect(run.progress.stages.workflows?.deleted).toBe(0) + }) + it('skips destructive hooks when a selected parent was restored', async () => { + let changedChildren = false + await boundedDelete(control(1), 'workflows', roots, roots.id, eq(roots.owner, 'one'), { + before: async (ids) => { + await db.update(roots).set({ owner: 'restored' }).where(eq(roots.id, ids[0])) + }, + beforeDelete: async () => { + changedChildren = true + }, + }) + expect(changedChildren).toBe(false) + expect(await count('children')).toBe(120) + }) + it('holds the parent lock throughout destructive child work', async () => { + await boundedDelete(control(1), 'workflows', roots, roots.id, eq(roots.id, 'a'), { + beforeDelete: async () => { + await expect( + runOutsideTransactionContext(() => + db.transaction(async (tx) => { + await tx.execute(sql`SET LOCAL lock_timeout = '100ms'`) + await tx.update(roots).set({ owner: 'restored' }).where(eq(roots.id, 'a')) + }) + ) + ).rejects.toThrow() + }, + }) + expect(await count('roots')).toBe(5) + }) it('aborts a blocked delete after the local lock timeout and preserves prior batches', async () => { let signalLocked!: () => void let release!: () => void @@ -197,3 +243,40 @@ describe('bounded metadata SQL against PostgreSQL', () => { ).toEqual(['protected', 'recent']) }) }) + +describe('large-value reference lifecycle locks', () => { + it.each(['metadata', 'legacy'] as const)( + 'protects a %s key until its reference commits', + async (kind) => { + const table = kind === 'metadata' ? 'execution_large_values' : 'workspace_files' + if (kind === 'metadata') + await db.execute(sql`INSERT INTO execution_large_values VALUES ('live-key','one',NULL)`) + else await db.execute(sql`INSERT INTO workspace_files VALUES ('live-key','execution',NULL)`) + await db.transaction(async (tx) => { + await lockLargeValueKeysForReference(tx, ['live-key']) + await expect( + runOutsideTransactionContext(() => + cleanupQuery(async (cleanupTx) => { + await cleanupTx.execute( + sql`SELECT key FROM ${sql.identifier(table)} WHERE key = 'live-key' FOR UPDATE` + ) + }) + ) + ).rejects.toThrow() + }) + await cleanupQuery(async (tx) => { + await tx.execute( + sql`UPDATE ${sql.identifier(table)} SET deleted_at = now() WHERE key = 'live-key'` + ) + }) + await expect( + db.transaction((tx) => lockLargeValueKeysForReference(tx, ['live-key'])) + ).rejects.toThrow('deleted large value') + } + ) + it('rejects reference creation after metadata has been purged', async () => { + await expect( + db.transaction((tx) => lockLargeValueKeysForReference(tx, ['missing-key'])) + ).rejects.toThrow('missing large value') + }) +}) diff --git a/apps/sim/lib/execution/payloads/large-array-manifest.test.ts b/apps/sim/lib/execution/payloads/large-array-manifest.test.ts index f0ef0e3468c..c5308d8414f 100644 --- a/apps/sim/lib/execution/payloads/large-array-manifest.test.ts +++ b/apps/sim/lib/execution/payloads/large-array-manifest.test.ts @@ -24,6 +24,11 @@ vi.mock('@/lib/uploads', () => ({ }, })) +vi.mock('@/lib/execution/payloads/large-value-metadata', () => ({ + registerLargeValueOwner: vi.fn().mockResolvedValue(true), + addLargeValueReference: vi.fn().mockResolvedValue(undefined), +})) + const TEST_CONTEXT = { workspaceId: 'workspace-1', workflowId: 'workflow-1', diff --git a/apps/sim/lib/execution/payloads/large-value-lock.ts b/apps/sim/lib/execution/payloads/large-value-lock.ts new file mode 100644 index 00000000000..f052353b66e --- /dev/null +++ b/apps/sim/lib/execution/payloads/large-value-lock.ts @@ -0,0 +1,44 @@ +import { executionLargeValues, workspaceFiles } from '@sim/db/schema' +import { chunkArray } from '@sim/utils/helpers' +import { and, asc, eq, inArray } from 'drizzle-orm' +import type { DbOrTx } from '@/lib/db/types' + +/** + * Reference writers hold shared locks until their transaction commits. Cleanup + * takes exclusive locks on the same rows before rechecking liveness and claiming + * a tombstone, so a new reference either protects the object or fails explicitly. + */ +export async function lockLargeValueKeysForReference( + tx: Pick, + keys: string[] +): Promise { + for (const batch of chunkArray([...new Set(keys)].sort(), 500)) { + const values = await tx + .select({ key: executionLargeValues.key, deletedAt: executionLargeValues.deletedAt }) + .from(executionLargeValues) + .where(inArray(executionLargeValues.key, batch)) + .orderBy(asc(executionLargeValues.key)) + .for('share') + const available = new Set() + for (const row of values) { + if (row.deletedAt) throw new Error('Cannot reference a deleted large value') + available.add(row.key) + } + const legacyKeys = batch.filter((key) => !available.has(key)) + if (legacyKeys.length > 0) { + const files = await tx + .select({ key: workspaceFiles.key, deletedAt: workspaceFiles.deletedAt }) + .from(workspaceFiles) + .where( + and(inArray(workspaceFiles.key, legacyKeys), eq(workspaceFiles.context, 'execution')) + ) + .orderBy(asc(workspaceFiles.key)) + .for('share') + for (const row of files) { + if (row.deletedAt) throw new Error('Cannot reference a deleted large value') + available.add(row.key) + } + } + if (available.size !== batch.length) throw new Error('Cannot reference a missing large value') + } +} diff --git a/apps/sim/lib/execution/payloads/large-value-metadata.test.ts b/apps/sim/lib/execution/payloads/large-value-metadata.test.ts index 4b1e4e7a45e..268c73133db 100644 --- a/apps/sim/lib/execution/payloads/large-value-metadata.test.ts +++ b/apps/sim/lib/execution/payloads/large-value-metadata.test.ts @@ -16,6 +16,10 @@ import { replaceLargeValueReferenceKeysWithClient, } from '@/lib/execution/payloads/large-value-metadata' +vi.mock('@/lib/execution/payloads/large-value-lock', () => ({ + lockLargeValueKeysForReference: vi.fn(), +})) + function largeValueKey(id: string, executionId = 'source-execution'): string { return `execution/workspace-1/workflow-1/${executionId}/large-value-lv_${id}.json` } @@ -323,18 +327,4 @@ describe('large value metadata', () => { tombstonesDeleted: 4, }) }) - - it('uses source-specific liveness when pruning stale references', async () => { - await pruneLargeValueMetadata({ - workspaceIds: ['workspace-1'], - tombstonesDeletedBefore: new Date('2026-01-01T00:00:00Z'), - batchSize: 10, - maxRowsPerTable: 100, - }) - - const [query] = dbChainMockFns.execute.mock.calls[0] ?? [] - const sqlText = Array.isArray(query?.strings) ? query.strings.join(' ') : '' - expect(sqlText).toContain("ref.source = 'execution_log'") - expect(sqlText).toContain("ref.source = 'paused_snapshot'") - }) }) diff --git a/apps/sim/lib/execution/payloads/large-value-metadata.ts b/apps/sim/lib/execution/payloads/large-value-metadata.ts index 7a188fef613..ed9113fb4c7 100644 --- a/apps/sim/lib/execution/payloads/large-value-metadata.ts +++ b/apps/sim/lib/execution/payloads/large-value-metadata.ts @@ -10,6 +10,7 @@ import { createLogger } from '@sim/logger' import { chunkArray } from '@sim/utils/helpers' import { and, eq, inArray, notInArray, sql } from 'drizzle-orm' import { collectLargeValueKeys } from '@/lib/execution/payloads/large-execution-value' +import { lockLargeValueKeysForReference } from '@/lib/execution/payloads/large-value-lock' const logger = createLogger('LargeValueMetadata') @@ -206,6 +207,7 @@ export async function registerLargeValueOwner( owner.workspaceId, referencedKeys ) + await lockLargeValueKeysForReference(tx, [owner.key, ...dependencyKeys]) if (dependencyKeys.length === 0) { return } @@ -245,34 +247,37 @@ export async function replaceLargeValueReferenceKeysWithClient( 'Large value reference set' ) - await client - .delete(executionLargeValueReferences) - .where( - and( - eq(executionLargeValueReferences.workspaceId, workspaceId), - eq(executionLargeValueReferences.executionId, executionId), - eq(executionLargeValueReferences.source, source) + await client.transaction(async (tx) => { + await lockLargeValueKeysForReference(tx, keys) + await tx + .delete(executionLargeValueReferences) + .where( + and( + eq(executionLargeValueReferences.workspaceId, workspaceId), + eq(executionLargeValueReferences.executionId, executionId), + eq(executionLargeValueReferences.source, source) + ) ) - ) - if (keys.length === 0) { - return - } + if (keys.length === 0) { + return + } - for (const keyChunk of chunkArray(keys, LARGE_VALUE_METADATA_WRITE_CHUNK_SIZE)) { - await client - .insert(executionLargeValueReferences) - .values( - keyChunk.map((key) => ({ - key, - workspaceId, - workflowId: workflowId ?? null, - executionId, - source, - })) - ) - .onConflictDoNothing() - } + for (const keyChunk of chunkArray(keys, LARGE_VALUE_METADATA_WRITE_CHUNK_SIZE)) { + await tx + .insert(executionLargeValueReferences) + .values( + keyChunk.map((key) => ({ + key, + workspaceId, + workflowId: workflowId ?? null, + executionId, + source, + })) + ) + .onConflictDoNothing() + } + }) } export async function addLargeValueReference( @@ -295,52 +300,54 @@ export async function addLargeValueReference( return } - const execDb = dbFor('exec') - const [existingRef] = await execDb - .select({ key: executionLargeValueReferences.key }) - .from(executionLargeValueReferences) - .where( - and( - eq(executionLargeValueReferences.workspaceId, workspaceId), - eq(executionLargeValueReferences.executionId, executionId), - eq(executionLargeValueReferences.source, source), - eq(executionLargeValueReferences.key, boundedKey) + await dbFor('exec').transaction(async (tx) => { + await lockLargeValueKeysForReference(tx, [boundedKey]) + const [existingRef] = await tx + .select({ key: executionLargeValueReferences.key }) + .from(executionLargeValueReferences) + .where( + and( + eq(executionLargeValueReferences.workspaceId, workspaceId), + eq(executionLargeValueReferences.executionId, executionId), + eq(executionLargeValueReferences.source, source), + eq(executionLargeValueReferences.key, boundedKey) + ) ) - ) - .limit(1) + .limit(1) - if (existingRef) { - return - } + if (existingRef) { + return + } - const existingRefs = await execDb - .select({ key: executionLargeValueReferences.key }) - .from(executionLargeValueReferences) - .where( - and( - eq(executionLargeValueReferences.workspaceId, workspaceId), - eq(executionLargeValueReferences.executionId, executionId), - eq(executionLargeValueReferences.source, source) + const existingRefs = await tx + .select({ key: executionLargeValueReferences.key }) + .from(executionLargeValueReferences) + .where( + and( + eq(executionLargeValueReferences.workspaceId, workspaceId), + eq(executionLargeValueReferences.executionId, executionId), + eq(executionLargeValueReferences.source, source) + ) ) - ) - .limit(MAX_LARGE_VALUE_REFERENCES_PER_SCOPE + 1) + .limit(MAX_LARGE_VALUE_REFERENCES_PER_SCOPE + 1) - if (existingRefs.length >= MAX_LARGE_VALUE_REFERENCES_PER_SCOPE) { - throw new Error( - `Large value reference set contains at least ${existingRefs.length} references, exceeding the limit of ${MAX_LARGE_VALUE_REFERENCES_PER_SCOPE}` - ) - } + if (existingRefs.length >= MAX_LARGE_VALUE_REFERENCES_PER_SCOPE) { + throw new Error( + `Large value reference set contains at least ${existingRefs.length} references, exceeding the limit of ${MAX_LARGE_VALUE_REFERENCES_PER_SCOPE}` + ) + } - await execDb - .insert(executionLargeValueReferences) - .values({ - key: boundedKey, - workspaceId, - workflowId: workflowId ?? null, - executionId, - source, - }) - .onConflictDoNothing() + await tx + .insert(executionLargeValueReferences) + .values({ + key: boundedKey, + workspaceId, + workflowId: workflowId ?? null, + executionId, + source, + }) + .onConflictDoNothing() + }) } export async function markLargeValuesDeleted( diff --git a/apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts b/apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts index f4387965a08..06b57176e62 100644 --- a/apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts +++ b/apps/sim/lib/execution/payloads/prune-metadata-sql.test.ts @@ -49,6 +49,13 @@ async function renderPruneStatements(workspaceIds: string[]): Promise { + it('uses source-specific liveness when pruning stale references', async () => { + const [references] = await renderPruneStatements(['ws-1']) + + expect(references.sql).toContain("ref.source = 'execution_log'") + expect(references.sql).toContain("ref.source = 'paused_snapshot'") + }) + for (const [label, ids] of [ ['multiple workspace ids', ['ws-1', 'ws-2']], ['a single workspace id', ['ws-only']], diff --git a/apps/sim/lib/knowledge/documents/storage-cleanup.ts b/apps/sim/lib/knowledge/documents/storage-cleanup.ts index 024cdac6c47..47492fef8cb 100644 --- a/apps/sim/lib/knowledge/documents/storage-cleanup.ts +++ b/apps/sim/lib/knowledge/documents/storage-cleanup.ts @@ -4,6 +4,7 @@ import { createLogger } from '@sim/logger' import { generateId } from '@sim/utils/id' import { and, eq, isNull, sql } from 'drizzle-orm' import { z } from 'zod' +import type { CleanupQuery } from '@/lib/cleanup/bounded' import type { OutboxHandler } from '@/lib/core/outbox/service' import { type ResourceOwner, @@ -160,15 +161,30 @@ function isMissingObject(error: unknown): boolean { * and a retry after an ambiguous object deletion treats absence as success. */ export const cleanupKnowledgeStorage: OutboxHandler = async (rawPayload, context) => { + await cleanupKnowledgeStorageBinding(rawPayload, context.signal) +} + +/** Reuses the binding lock shared by document creation and storage restoration. */ +export async function cleanupKnowledgeStorageBinding( + rawPayload: unknown, + signal: AbortSignal, + query?: CleanupQuery +): Promise { const payload = cleanupPayloadSchema.parse(rawPayload) if (!isKnowledgeBaseOwnedStorageKey(payload.key)) { throw new Error('Knowledge storage cleanup requires a knowledge-base key') } assertCleanupOwner(payload) - context.signal.throwIfAborted() - await db.transaction(async (tx) => { - await tx.execute(sql`SET LOCAL lock_timeout = '5s'`) - await tx.execute(sql`SET LOCAL statement_timeout = '20s'`) + signal.throwIfAborted() + const execute: CleanupQuery = + query ?? + ((callback) => + db.transaction(async (tx) => { + await tx.execute(sql`SET LOCAL lock_timeout = '5s'`) + await tx.execute(sql`SET LOCAL statement_timeout = '20s'`) + return callback(tx) + })) + return execute(async (tx) => { const [binding] = await tx .select({ id: workspaceFiles.id, @@ -183,7 +199,7 @@ export const cleanupKnowledgeStorage: OutboxHandler = async (rawPayload, context .where(and(eq(workspaceFiles.id, payload.fileId), isNull(workspaceFiles.deletedAt))) .for('update') .limit(1) - context.signal.throwIfAborted() + signal.throwIfAborted() if ( !binding || binding.key !== payload.key || @@ -191,17 +207,17 @@ export const cleanupKnowledgeStorage: OutboxHandler = async (rawPayload, context binding.contentUpdatedAt.toISOString() !== payload.contentUpdatedAt || !sameCleanupOwner(binding, payload) ) - return + return false const [reference] = await tx .select({ id: document.id }) .from(document) .where(eq(document.storageKey, payload.key)) .limit(1) - if (reference) return + if (reference) return false - const signal = AbortSignal.any([context.signal, AbortSignal.timeout(STORAGE_TIMEOUT_MS)]) - signal.throwIfAborted() + const storageSignal = AbortSignal.any([signal, AbortSignal.timeout(STORAGE_TIMEOUT_MS)]) + storageSignal.throwIfAborted() const object = payload.uploadId ? await checkpointIo( () => @@ -210,22 +226,23 @@ export const cleanupKnowledgeStorage: OutboxHandler = async (rawPayload, context key: payload.key, context: 'knowledge-base', }), - signal + storageSignal ) : undefined if (!payload.uploadId || object?.uploadId === payload.uploadId) { try { - await deleteFile({ key: payload.key, context: 'knowledge-base', signal }) + await deleteFile({ key: payload.key, context: 'knowledge-base', signal: storageSignal }) } catch (error) { - signal.throwIfAborted() + storageSignal.throwIfAborted() if (!isMissingObject(error)) throw error } } - signal.throwIfAborted() + storageSignal.throwIfAborted() const deleted = await deleteFileMetadataByIdentity( { ...binding, context: 'knowledge-base' }, tx ) if (!deleted) throw new Error('Knowledge storage cleanup lost its metadata identity') + return true }) } From a18eec1e7cfd583bacbcd216ee6f392f80e84281 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Mon, 14 Sep 2026 20:11:09 -0700 Subject: [PATCH 3/4] fix(cleanup): persist storage retries and complete test fixtures --- .../app/api/webhooks/outbox/process/route.ts | 2 + apps/sim/background/cleanup-bounded.test.ts | 35 ++++++- apps/sim/background/cleanup-logs-bounded.ts | 55 +++++++---- .../cleanup-soft-deletes-bounded.ts | 48 ++++++---- .../variables/variables-handler.test.ts | 6 ++ apps/sim/executor/orchestrators/loop.test.ts | 6 ++ .../variables/resolvers/block.test.ts | 6 ++ apps/sim/lib/cleanup/bounded-cleanup.md | 4 +- .../cleanup/bounded.postgres.integration.ts | 72 ++++++++++++++- apps/sim/lib/cleanup/storage-outbox.ts | 92 +++++++++++++++++++ .../execute-request.test.ts | 6 ++ .../utils/user-file-base64.server.test.ts | 6 ++ .../lib/workflows/streaming/streaming.test.ts | 6 ++ 13 files changed, 302 insertions(+), 42 deletions(-) create mode 100644 apps/sim/lib/cleanup/storage-outbox.ts diff --git a/apps/sim/app/api/webhooks/outbox/process/route.ts b/apps/sim/app/api/webhooks/outbox/process/route.ts index f79d06a4e28..c9e9340ea0c 100644 --- a/apps/sim/app/api/webhooks/outbox/process/route.ts +++ b/apps/sim/app/api/webhooks/outbox/process/route.ts @@ -9,6 +9,7 @@ import { enterpriseOwnerClaimOutboxHandlers } from '@/lib/billing/enterprise-own import { enterpriseIssuanceOutboxHandlers } from '@/lib/billing/enterprise-provisioning' import { membershipBillingOutboxHandlers } from '@/lib/billing/organizations/membership-reconciliation' import { billingOutboxHandlers } from '@/lib/billing/webhooks/outbox-handlers' +import { retentionStorageOutboxHandlers } from '@/lib/cleanup/storage-outbox' import { processOutboxEvents } from '@/lib/core/outbox/service' import { DeadlineExceededError } from '@/lib/core/utils/deadline' import { generateRequestId } from '@/lib/core/utils/request' @@ -33,6 +34,7 @@ export const dynamic = 'force-dynamic' export const maxDuration = 800 const handlers = { + ...retentionStorageOutboxHandlers, ...slackSearchOutboxHandlers, ...adminInvitationOperationOutboxHandlers, ...adminMemberOperationOutboxHandlers, diff --git a/apps/sim/background/cleanup-bounded.test.ts b/apps/sim/background/cleanup-bounded.test.ts index 3e6deeff1f2..9a564443946 100644 --- a/apps/sim/background/cleanup-bounded.test.ts +++ b/apps/sim/background/cleanup-bounded.test.ts @@ -7,6 +7,7 @@ import { schemaMock, } from '@sim/testing' import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { OutboxHandlerRegistry } from '@/lib/core/outbox/service' const { storage, prepareChat, executeChat, hardDelete, billing, decrement, reRoot } = vi.hoisted( () => ({ @@ -19,6 +20,35 @@ const { storage, prepareChat, executeChat, hardDelete, billing, decrement, reRoo reRoot: vi.fn(), }) ) +const outbox = vi.hoisted(() => new Map()) +vi.mock('@/lib/core/outbox/service', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + enqueueOutboxEvent: vi.fn(async (...args: Parameters) => { + const id = await actual.enqueueOutboxEvent(...args) + outbox.set(id, { eventType: args[1], payload: args[2] }) + return id + }), + processOutboxEventById: vi.fn(async (id: string, handlers: OutboxHandlerRegistry) => { + const event = outbox.get(id) + if (!event) throw new Error('Missing test outbox event') + try { + await handlers[event.eventType](event.payload, { + eventId: id, + eventType: event.eventType, + attempts: 0, + maxAttempts: 48, + signal: new AbortController().signal, + checkpointPayload: async () => {}, + }) + return 'completed' + } catch { + return 'pending' + } + }), + } +}) vi.mock('@/background/cleanup-logs', () => ({ legacyLargeValuePredicate: vi.fn() })) vi.mock('@/background/cleanup-soft-deletes', () => ({ reRootActiveFolderChildrenUnguarded: reRoot, @@ -57,6 +87,7 @@ function control(type: CleanupType, dryRun = true, limit = 1) { beforeEach(() => { vi.clearAllMocks() resetDbChainMock() + outbox.clear() storage.mockResolvedValue({ deleted: 1, failed: [] }) prepareChat.mockResolvedValue({ execute: executeChat }) }) @@ -125,7 +156,7 @@ describe('requested cleanup stages', () => { dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'one', files: [{ key: 'blob' }] }]) storage.mockResolvedValue({ deleted: 0, failed: [{ key: 'blob', error: 'unavailable' }] }) const run = control('workflowLogs', false) - await expect(runBoundedLogScope(scope, run)).rejects.toThrow('storage deletions failed') + await expect(runBoundedLogScope(scope, run)).rejects.toThrow('storage cleanup is incomplete') expect(dbChainMockFns.delete).toHaveBeenCalledOnce() expect(run.progress.stages.workflowLogs).toMatchObject({ selected: 1, @@ -215,7 +246,7 @@ describe('bounded file billing', () => { { id: 'file-one', key: 'blob', context: 'workspace', workspaceId: 'ws-one', sizeBytes: 100 }, ]) billing.mockResolvedValue({ workspaceId: 'ws-one' }) - dbChainMockFns.returning.mockResolvedValue([{ id: 'file-one', sizeBytes: 40 }]) + dbChainMockFns.returning.mockResolvedValue([{ id: 'file-one', key: 'blob', sizeBytes: 40 }]) const run = control('files', false) await runBoundedSoftDeleteScope({ ...scope, workspaceIds: ['ws-one', 'ws-two'] }, run) expect(decrement).toHaveBeenCalledWith( diff --git a/apps/sim/background/cleanup-logs-bounded.ts b/apps/sim/background/cleanup-logs-bounded.ts index 40d14698094..b6a3bc12302 100644 --- a/apps/sim/background/cleanup-logs-bounded.ts +++ b/apps/sim/background/cleanup-logs-bounded.ts @@ -12,7 +12,10 @@ import type { CleanupJobPayload } from '@/lib/billing/cleanup-dispatcher' import type { BoundedCleanup } from '@/lib/cleanup/bounded' import { boundedDelete } from '@/lib/cleanup/bounded-delete' import { pruneBoundedLargeValueMetadata } from '@/lib/cleanup/bounded-large-value-metadata' -import { deleteBoundedStorage, tombstoneBoundedFiles } from '@/lib/cleanup/bounded-storage' +import { + enqueueRetentionStorageCleanup, + processRetentionStorageCleanup, +} from '@/lib/cleanup/storage-outbox' import { LIVE_PAUSED_REFERENCE_STATUSES, unreferencedLargeValuePredicate, @@ -51,8 +54,8 @@ export async function runBoundedLogScope(payload: CleanupJobPayload, control: Bo ), (row) => row.id, async (rows) => { - const deleted = await control.query(async (tx) => - tx + const { deleted, events } = await control.query(async (tx) => { + const deleted = await tx .delete(workflowExecutionLogs) .where( and( @@ -64,19 +67,28 @@ export async function runBoundedLogScope(payload: CleanupJobPayload, control: Bo ) ) .returning({ id: workflowExecutionLogs.id, files: workflowExecutionLogs.files }) - ) - await control.deleted('workflowLogs', deleted.length) - for (const row of deleted) { - const keys = Array.isArray(row.files) - ? row.files.flatMap((file) => - file && typeof file === 'object' && 'key' in file && typeof file.key === 'string' - ? [file.key] - : [] + const keys = deleted.flatMap((row) => + Array.isArray(row.files) + ? row.files.flatMap((file) => + file && typeof file === 'object' && 'key' in file && typeof file.key === 'string' + ? [file.key] + : [] + ) + : [] + ) + const events = isUsingCloudStorage() + ? await enqueueRetentionStorageCleanup( + tx, + keys, + 'execution', + control.options.batchSize, + true ) : [] - await deleteBoundedStorage(control, 'workflowLogs', keys, 'execution') - if (isUsingCloudStorage()) await tombstoneBoundedFiles(control, keys) - } + return { deleted, events } + }) + await control.deleted('workflowLogs', deleted.length) + await processRetentionStorageCleanup(control, 'workflowLogs', events) } ) await boundedDelete( @@ -123,7 +135,7 @@ export async function runBoundedLogScope(payload: CleanupJobPayload, control: Bo async (rows) => { if (!isUsingCloudStorage()) return const selectedKeys = rows.map((row) => row.key) - const keys = await control.query(async (tx) => { + const { keys, events } = await control.query(async (tx) => { await tx .select({ key: table.key }) .from(table) @@ -135,11 +147,18 @@ export async function runBoundedLogScope(payload: CleanupJobPayload, control: Bo .set({ deletedAt: new Date() }) .where(and(eligible, inArray(table.key, selectedKeys))) .returning({ key: table.key }) - return claimed.map((row) => row.key) + const keys = claimed.map((row) => row.key) + const events = await enqueueRetentionStorageCleanup( + tx, + keys, + 'execution', + control.options.batchSize, + true + ) + return { keys, events } }) - await deleteBoundedStorage(control, type, keys, 'execution') + await processRetentionStorageCleanup(control, type, events) await control.deleted(type, keys.length) - await tombstoneBoundedFiles(control, keys) } ) } diff --git a/apps/sim/background/cleanup-soft-deletes-bounded.ts b/apps/sim/background/cleanup-soft-deletes-bounded.ts index bc63b8187c2..419c20d02da 100644 --- a/apps/sim/background/cleanup-soft-deletes-bounded.ts +++ b/apps/sim/background/cleanup-soft-deletes-bounded.ts @@ -21,7 +21,6 @@ import { } from '@/lib/billing/storage' import { type BoundedCleanup, setCleanupTimeouts } from '@/lib/cleanup/bounded' import { boundedDelete } from '@/lib/cleanup/bounded-delete' -import { deleteBoundedStorage } from '@/lib/cleanup/bounded-storage' import type { CleanupType } from '@/lib/cleanup/bounded-types' import { prepareChatCleanup } from '@/lib/cleanup/chat-cleanup' import { @@ -29,9 +28,13 @@ import { cleanupOwnerCondition, resolveCleanupOwnerScope, } from '@/lib/cleanup/resource-scope' +import { + enqueueRetentionStorageCleanup, + processRetentionStorageCleanup, +} from '@/lib/cleanup/storage-outbox' import { hardDeleteDocuments } from '@/lib/knowledge/documents/service' import { cleanupKnowledgeStorageBinding } from '@/lib/knowledge/documents/storage-cleanup' -import type { StorageContext } from '@/lib/uploads' +import { isUsingCloudStorage, type StorageContext } from '@/lib/uploads' import { getWorkspaceFileSize } from '@/lib/uploads/shared/types' import { reRootActiveFolderChildrenUnguarded } from '@/background/cleanup-soft-deletes' @@ -207,8 +210,8 @@ async function cleanupFiles(control: BoundedCleanup, scope: CleanupOwnerScope, c ), (row) => row.id, async (rows) => { - const deleted = await control.query(async (tx) => - tx + const { deleted, events } = await control.query(async (tx) => { + const deleted = await tx .delete(workspaceFile) .where( and( @@ -220,14 +223,18 @@ async function cleanupFiles(control: BoundedCleanup, scope: CleanupOwnerScope, c ) ) .returning({ id: workspaceFile.id, key: workspaceFile.key }) - ) + const events = isUsingCloudStorage() + ? await enqueueRetentionStorageCleanup( + tx, + deleted.map((row) => row.key), + 'workspace', + control.options.batchSize + ) + : [] + return { deleted, events } + }) await control.deleted('legacyFiles', deleted.length) - await deleteBoundedStorage( - control, - 'legacyFiles', - deleted.map((row) => row.key), - 'workspace' - ) + await processRetentionStorageCleanup(control, 'legacyFiles', events) } ) } @@ -289,21 +296,24 @@ async function cleanupFiles(control: BoundedCleanup, scope: CleanupOwnerScope, c billing, deleted.reduce((sum, file) => sum + getWorkspaceFileSize(file), 0) ) - return deleted + const events = isUsingCloudStorage() + ? await enqueueRetentionStorageCleanup( + tx, + deleted.map((file) => file.key), + row.context as StorageContext, + control.options.batchSize + ) + : [] + return { deleted, events } } - const deleted = billing + const { deleted, events } = billing ? await db.transaction(async (tx) => { await setCleanupTimeouts(tx) return remove(tx) }) : await control.query(remove) await control.deleted('files', deleted.length) - await deleteBoundedStorage( - control, - 'files', - deleted.map((file) => file.key), - row.context as StorageContext - ) + await processRetentionStorageCleanup(control, 'files', events) } } ) diff --git a/apps/sim/executor/handlers/variables/variables-handler.test.ts b/apps/sim/executor/handlers/variables/variables-handler.test.ts index 086d5bf5cf0..aee2cc9e1dd 100644 --- a/apps/sim/executor/handlers/variables/variables-handler.test.ts +++ b/apps/sim/executor/handlers/variables/variables-handler.test.ts @@ -13,6 +13,12 @@ const { mockUploadFile } = vi.hoisted(() => ({ mockUploadFile: vi.fn(), })) +vi.mock('@/lib/execution/payloads/large-value-metadata', async (importOriginal) => ({ + ...(await importOriginal()), + registerLargeValueOwner: vi.fn().mockResolvedValue(true), + addLargeValueReference: vi.fn().mockResolvedValue(undefined), +})) + vi.mock('@/lib/uploads', () => ({ StorageService: { uploadFile: mockUploadFile, diff --git a/apps/sim/executor/orchestrators/loop.test.ts b/apps/sim/executor/orchestrators/loop.test.ts index 67af5313573..67b8ee15988 100644 --- a/apps/sim/executor/orchestrators/loop.test.ts +++ b/apps/sim/executor/orchestrators/loop.test.ts @@ -22,6 +22,12 @@ const mockLogger = vi.mocked(createLogger).mock.calls.findIndex(([name]) => name === 'LoopOrchestrator') ].value +vi.mock('@/lib/execution/payloads/large-value-metadata', async (importOriginal) => ({ + ...(await importOriginal()), + registerLargeValueOwner: vi.fn().mockResolvedValue(true), + addLargeValueReference: vi.fn().mockResolvedValue(undefined), +})) + vi.mock('@/lib/execution/isolated-vm', () => ({ executeInIsolatedVM: mockExecuteInIsolatedVM, })) diff --git a/apps/sim/executor/variables/resolvers/block.test.ts b/apps/sim/executor/variables/resolvers/block.test.ts index c972df2afc7..ef9b28102ec 100644 --- a/apps/sim/executor/variables/resolvers/block.test.ts +++ b/apps/sim/executor/variables/resolvers/block.test.ts @@ -7,6 +7,12 @@ import { navigatePathAsync } from '@/executor/variables/resolvers/reference-asyn import { BlockResolver } from './block' import { RESOLVED_EMPTY, type ResolutionContext } from './reference' +vi.mock('@/lib/execution/payloads/large-value-metadata', async (importOriginal) => ({ + ...(await importOriginal()), + registerLargeValueOwner: vi.fn().mockResolvedValue(true), + addLargeValueReference: vi.fn().mockResolvedValue(undefined), +})) + vi.mock('@/lib/uploads/server/metadata', () => ({ insertImmutableFileMetadata: vi.fn().mockResolvedValue({ id: 'execution-payload-file' }), insertFileMetadata: vi.fn().mockResolvedValue({ id: 'execution-payload-file' }), diff --git a/apps/sim/lib/cleanup/bounded-cleanup.md b/apps/sim/lib/cleanup/bounded-cleanup.md index ec7ca54671f..18de9bef090 100644 --- a/apps/sim/lib/cleanup/bounded-cleanup.md +++ b/apps/sim/lib/cleanup/bounded-cleanup.md @@ -53,11 +53,11 @@ Limits count **selected roots**, including roots restored before deletion. They - Bounded dispatch sets one attempt and a 180-second hard maximum. The worker stops starting new root batches after 120 seconds; cancellable child preparation also observes that work deadline. - Cleanup SQL uses transaction-local **500ms lock_timeout** and **5s statement_timeout**. The billable-file delete and storage decrement remain atomic. Orphan knowledge-base storage cleanup reuses the binding lock held by document creation, with a 15-second storage deadline. Large-value reference writers lock the value before registering references; cleanup locks and rechecks it before claiming a tombstone. - Trigger `cleanup` metadata reports each requested type's `selected`, `deleted`, `skipped`, `filesDeleted`, and `filesFailed`, plus stage, duration, and stop reason: `budgets_exhausted`, `scopes_exhausted`, `time_budget`, or `failed`. -- A failure stops subsequent stages, preserves completed progress, and fails the run. External deletion is not transactional with Postgres. A hard process termination may leave the last metadata checkpoint behind actual effects. Do not blindly replay failed runs: inspect the failed stage and any external work already completed. Log and file storage is removed only for rows returned by the guarded delete. A storage failure after that commit can leave orphaned objects; a failed large-value storage deletion can leave a tombstone with remaining bytes. This change does not add recovery for those gaps or for chat backend/storage cleanup failures after parent deletion. Inspect failed runs before resubmitting. +- A failure stops subsequent stages, preserves completed progress, and fails the run. External deletion is not transactional with Postgres. A hard process termination may leave the last metadata checkpoint behind actual effects. Do not blindly replay failed runs: inspect the failed stage and any external work already completed. Log and file storage is removed only for rows returned by the guarded delete. Their keys, and claimed large-value keys, enter `retention.storage.cleanup` outbox events in the same database transaction. The run attempts those events immediately; the existing outbox worker retries failures without selecting more roots. Inspect pending/dead-letter events when a run fails. Chat backend/storage cleanup still has its existing post-parent-delete recovery gap. ## Rollout -1. Deploy Trigger workers first, then the API. Keep the existing production cleanup schedules disabled. +1. Deploy Trigger workers first, then the API. Keep the existing production cleanup schedules disabled. Ensure the existing outbox processor is running so persisted storage failures can retry. 2. Let old cleanup runs finish or cancel them before manual draining. Old queued jobs may retain their previous queue/version settings; the new queue cannot serialize against those runs. 3. Run a preview, then one small delete invocation. Wait for the Trigger run to finish. Check database CPU, query latency, lock waits, replication lag, and application errors against their normal baseline. 4. Repeat with a fresh request ID. Increase either the per-call budget or call frequency gradually, holding the other steady. Pause submissions on timeouts, failures, or database/application degradation. Avoid overlapping caller loops even though the Trigger queue serializes these jobs. diff --git a/apps/sim/lib/cleanup/bounded.postgres.integration.ts b/apps/sim/lib/cleanup/bounded.postgres.integration.ts index b958ccd5bd4..da9be851786 100644 --- a/apps/sim/lib/cleanup/bounded.postgres.integration.ts +++ b/apps/sim/lib/cleanup/bounded.postgres.integration.ts @@ -1,12 +1,21 @@ import { db, dbFor, runOutsideTransactionContext } from '@sim/db' import { eq, sql } from 'drizzle-orm' import { pgTable, text } from 'drizzle-orm/pg-core' -import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest' +import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' import { BoundedCleanup, cleanupQuery } from '@/lib/cleanup/bounded' import { boundedDelete } from '@/lib/cleanup/bounded-delete' import { pruneBoundedLargeValueMetadata } from '@/lib/cleanup/bounded-large-value-metadata' +import { + enqueueRetentionStorageCleanup, + processRetentionStorageCleanup, + retentionStorageOutboxHandlers, +} from '@/lib/cleanup/storage-outbox' +import { processOutboxEventById } from '@/lib/core/outbox/service' import { lockLargeValueKeysForReference } from '@/lib/execution/payloads/large-value-lock' +const { deleteStorageFiles } = vi.hoisted(() => ({ deleteStorageFiles: vi.fn() })) +vi.mock('@/lib/uploads', () => ({ StorageService: { deleteFiles: deleteStorageFiles } })) + const url = new URL(process.env.DATABASE_URL ?? '') if (url.hostname !== '127.0.0.1' || url.pathname !== '/bounded_cleanup_test') { throw new Error('This suite requires an isolated local bounded_cleanup_test database') @@ -18,6 +27,12 @@ const roots = pgTable('cleanup_fixture_roots', { const client = dbFor('cleanup') beforeAll(async () => { + await db.execute(sql`CREATE TABLE outbox_event ( + id text PRIMARY KEY, event_type text NOT NULL, payload json NOT NULL, + status text NOT NULL DEFAULT 'pending', attempts integer NOT NULL DEFAULT 0, + max_attempts integer NOT NULL DEFAULT 10, available_at timestamp NOT NULL DEFAULT now(), + locked_at timestamp, last_error text, created_at timestamp NOT NULL DEFAULT now(), processed_at timestamp + )`) await db.execute( sql`CREATE TABLE execution_large_value_references (key text, workspace_id text, execution_id text, source text, PRIMARY KEY(key, execution_id, source))` ) @@ -41,6 +56,8 @@ beforeAll(async () => { ) }) beforeEach(async () => { + deleteStorageFiles.mockReset() + await db.execute(sql`TRUNCATE outbox_event`) await db.execute( sql`TRUNCATE execution_large_value_references, execution_large_value_dependencies, execution_large_values, workflow_execution_logs, paused_executions, workspace_files` ) @@ -56,6 +73,7 @@ beforeEach(async () => { ) }) afterAll(async () => { + await db.execute(sql`DROP TABLE IF EXISTS outbox_event`) await db.execute( sql`DROP TABLE IF EXISTS execution_large_value_references, execution_large_value_dependencies, execution_large_values, workflow_execution_logs, paused_executions, workspace_files` ) @@ -280,3 +298,55 @@ describe('large-value reference lifecycle locks', () => { ).rejects.toThrow('missing large value') }) }) + +describe('durable retention storage cleanup', () => { + it('rolls back cleanup intents with the root deletion', async () => { + await expect( + cleanupQuery(async (tx) => { + await tx.delete(roots).where(eq(roots.id, 'a')) + await enqueueRetentionStorageCleanup(tx, ['blob'], 'execution', 1) + throw new Error('abort transaction') + }) + ).rejects.toThrow('abort transaction') + expect(await count('roots')).toBe(6) + expect((await db.execute(sql`SELECT id FROM outbox_event`)).length).toBe(0) + expect(deleteStorageFiles).not.toHaveBeenCalled() + }) + it('retries storage from the outbox after the root is gone', async () => { + const [eventId] = await cleanupQuery(async (tx) => { + await tx.delete(roots).where(eq(roots.id, 'a')) + return enqueueRetentionStorageCleanup(tx, ['blob'], 'execution', 1) + }) + deleteStorageFiles.mockResolvedValueOnce({ + deleted: 0, + failed: [{ key: 'blob', error: 'offline' }], + }) + await expect( + processRetentionStorageCleanup(control(1), 'workflows', [eventId]) + ).rejects.toThrow('incomplete: pending') + expect(await count('roots')).toBe(5) + const [pending] = await db.execute<{ status: string; payload: { keys: string[] } }>( + sql`SELECT status, payload FROM outbox_event WHERE id = ${eventId}` + ) + expect(pending.status).toBe('pending') + expect(pending.payload.keys).toEqual(['blob']) + await db.execute( + sql`UPDATE outbox_event SET available_at = now() - interval '1 second' WHERE id = ${eventId}` + ) + deleteStorageFiles.mockResolvedValueOnce({ deleted: 1, failed: [] }) + await expect(processOutboxEventById(eventId, retentionStorageOutboxHandlers)).resolves.toBe( + 'completed' + ) + expect(deleteStorageFiles).toHaveBeenCalledTimes(2) + expect(await count('roots')).toBe(5) + }) + it('bounds and deduplicates the persisted key batches', async () => { + await cleanupQuery((tx) => + enqueueRetentionStorageCleanup(tx, ['a', 'b', 'a', 'c'], 'workspace', 2) + ) + const events = await db.execute<{ payload: { keys: string[] } }>( + sql`SELECT payload FROM outbox_event ORDER BY created_at, id` + ) + expect(events.map((event) => event.payload.keys.length).sort()).toEqual([1, 2]) + }) +}) diff --git a/apps/sim/lib/cleanup/storage-outbox.ts b/apps/sim/lib/cleanup/storage-outbox.ts new file mode 100644 index 00000000000..8d096a3efc0 --- /dev/null +++ b/apps/sim/lib/cleanup/storage-outbox.ts @@ -0,0 +1,92 @@ +import { workspaceFiles } from '@sim/db/schema' +import { chunkArray } from '@sim/utils/helpers' +import { and, inArray, isNull } from 'drizzle-orm' +import { z } from 'zod' +import { type BoundedCleanup, type CleanupTransaction, cleanupQuery } from '@/lib/cleanup/bounded' +import type { CleanupType } from '@/lib/cleanup/bounded-types' +import { + enqueueOutboxEvent, + type OutboxHandler, + processOutboxEventById, +} from '@/lib/core/outbox/service' +import type { StorageContext } from '@/lib/uploads/shared/types' + +const EVENT = 'retention.storage.cleanup' +const payloadSchema = z + .object({ + keys: z.array(z.string().min(1)).min(1).max(500), + context: z.enum([ + 'knowledge-base', + 'chat', + 'copilot', + 'mothership', + 'execution', + 'workspace', + 'table-import', + 'profile-pictures', + 'og-images', + 'logs', + 'workspace-logos', + 'organization-logos', + ]), + tombstoneFiles: z.boolean(), + }) + .strict() + +function handler(onResult?: (deleted: number, failed: number) => Promise): OutboxHandler { + return async (rawPayload, context) => { + const payload = payloadSchema.parse(rawPayload) + context.signal.throwIfAborted() + const { StorageService } = await import('@/lib/uploads') + let result: Awaited> + try { + result = await StorageService.deleteFiles(payload.keys, payload.context) + } catch (error) { + await onResult?.(0, payload.keys.length) + throw error + } + await onResult?.(result.deleted, result.failed.length) + if (result.failed.length > 0) throw new Error('Retention storage deletions failed') + context.signal.throwIfAborted() + if (payload.tombstoneFiles) { + await cleanupQuery(async (tx) => { + await tx + .update(workspaceFiles) + .set({ deletedAt: new Date() }) + .where(and(inArray(workspaceFiles.key, payload.keys), isNull(workspaceFiles.deletedAt))) + }) + } + } +} + +export const retentionStorageOutboxHandlers = { [EVENT]: handler() } + +/** Persist immutable keys in the same transaction that deletes or claims their roots. */ +export async function enqueueRetentionStorageCleanup( + tx: Pick, + keys: string[], + context: StorageContext, + batchSize: number, + tombstoneFiles = false +): Promise { + const events: string[] = [] + for (const batch of chunkArray([...new Set(keys)], batchSize)) { + const payload = payloadSchema.parse({ keys: batch, context, tombstoneFiles }) + events.push(await enqueueOutboxEvent(tx, EVENT, payload, { maxAttempts: 48 })) + } + return events +} + +/** Attempt only this batch's durable events; the existing outbox worker retries failures. */ +export async function processRetentionStorageCleanup( + control: BoundedCleanup, + type: CleanupType, + eventIds: string[] +): Promise { + const handlers = { [EVENT]: handler((deleted, failed) => control.files(type, deleted, failed)) } + for (const eventId of eventIds) { + const result = await processOutboxEventById(eventId, handlers) + if (result !== 'completed') + throw new Error(`Retention storage cleanup is incomplete: ${result}`) + } +} diff --git a/apps/sim/lib/function-execution/execute-request.test.ts b/apps/sim/lib/function-execution/execute-request.test.ts index 36ceb0a0e59..e1d307f91d2 100644 --- a/apps/sim/lib/function-execution/execute-request.test.ts +++ b/apps/sim/lib/function-execution/execute-request.test.ts @@ -71,6 +71,12 @@ const { mockRenderedMountContributors: vi.fn(), })) +vi.mock('@/lib/execution/payloads/large-value-metadata', async (importOriginal) => ({ + ...(await importOriginal()), + registerLargeValueOwner: vi.fn().mockResolvedValue(true), + addLargeValueReference: vi.fn().mockResolvedValue(undefined), +})) + vi.mock('@/lib/core/security/encryption', () => ({ decryptSecret: mockDecryptSecret, encryptSecret: mockEncryptSecret, diff --git a/apps/sim/lib/uploads/utils/user-file-base64.server.test.ts b/apps/sim/lib/uploads/utils/user-file-base64.server.test.ts index 49ac99226c9..64f0ad7db82 100644 --- a/apps/sim/lib/uploads/utils/user-file-base64.server.test.ts +++ b/apps/sim/lib/uploads/utils/user-file-base64.server.test.ts @@ -41,6 +41,12 @@ const mockGetRedisClient = redisConfigMockFns.mockGetRedisClient afterAll(resetRedisConfigMock) +vi.mock('@/lib/execution/payloads/large-value-metadata', async (importOriginal) => ({ + ...(await importOriginal()), + registerLargeValueOwner: vi.fn().mockResolvedValue(true), + addLargeValueReference: vi.fn().mockResolvedValue(undefined), +})) + vi.mock('@/lib/uploads', () => ({ StorageService: { downloadFile: mockDownloadFile, diff --git a/apps/sim/lib/workflows/streaming/streaming.test.ts b/apps/sim/lib/workflows/streaming/streaming.test.ts index 41aa41f5a33..c7e028eb0b9 100644 --- a/apps/sim/lib/workflows/streaming/streaming.test.ts +++ b/apps/sim/lib/workflows/streaming/streaming.test.ts @@ -30,6 +30,12 @@ const { mockDownloadFile } = vi.hoisted(() => ({ mockDownloadFile: vi.fn(), })) +vi.mock('@/lib/execution/payloads/large-value-metadata', async (importOriginal) => ({ + ...(await importOriginal()), + registerLargeValueOwner: vi.fn().mockResolvedValue(true), + addLargeValueReference: vi.fn().mockResolvedValue(undefined), +})) + vi.mock('@/lib/uploads', () => ({ StorageService: { downloadFile: mockDownloadFile, From 0ccd2f1d56e25408dc1bfa987fe42765ced8b891 Mon Sep 17 00:00:00 2001 From: Theodore Li Date: Mon, 14 Sep 2026 20:27:01 -0700 Subject: [PATCH 4/4] fix(cleanup): guard storage retries by file generation --- apps/sim/background/cleanup-bounded.test.ts | 16 +-- .../variables/variables-handler.test.ts | 3 +- apps/sim/executor/orchestrators/loop.test.ts | 3 +- .../variables/resolvers/block.test.ts | 3 +- apps/sim/lib/cleanup/bounded-cleanup.md | 2 +- .../cleanup/bounded.postgres.integration.ts | 100 +++++++++++--- apps/sim/lib/cleanup/bounded.ts | 2 +- apps/sim/lib/cleanup/storage-outbox.ts | 122 ++++++++++++++---- .../execute-request.test.ts | 3 +- .../utils/user-file-base64.server.test.ts | 3 +- .../lib/workflows/streaming/streaming.test.ts | 3 +- 11 files changed, 196 insertions(+), 64 deletions(-) diff --git a/apps/sim/background/cleanup-bounded.test.ts b/apps/sim/background/cleanup-bounded.test.ts index 9a564443946..ca9a4bf6c73 100644 --- a/apps/sim/background/cleanup-bounded.test.ts +++ b/apps/sim/background/cleanup-bounded.test.ts @@ -21,13 +21,11 @@ const { storage, prepareChat, executeChat, hardDelete, billing, decrement, reRoo }) ) const outbox = vi.hoisted(() => new Map()) -vi.mock('@/lib/core/outbox/service', async (importOriginal) => { - const actual = await importOriginal() +vi.mock('@/lib/core/outbox/service', () => { return { - ...actual, - enqueueOutboxEvent: vi.fn(async (...args: Parameters) => { - const id = await actual.enqueueOutboxEvent(...args) - outbox.set(id, { eventType: args[1], payload: args[2] }) + enqueueOutboxEvent: vi.fn(async (_tx: unknown, eventType: string, payload: unknown) => { + const id = `test-outbox-${outbox.size}` + outbox.set(id, { eventType, payload }) return id }), processOutboxEventById: vi.fn(async (id: string, handlers: OutboxHandlerRegistry) => { @@ -55,7 +53,7 @@ vi.mock('@/background/cleanup-soft-deletes', () => ({ })) vi.mock('@/lib/uploads', () => ({ isUsingCloudStorage: () => true, - StorageService: { deleteFiles: storage }, + StorageService: { deleteFile: storage }, })) vi.mock('@/lib/cleanup/chat-cleanup', () => ({ prepareChatCleanup: prepareChat })) vi.mock('@/lib/knowledge/documents/service', () => ({ hardDeleteDocuments: hardDelete })) @@ -88,7 +86,7 @@ beforeEach(() => { vi.clearAllMocks() resetDbChainMock() outbox.clear() - storage.mockResolvedValue({ deleted: 1, failed: [] }) + storage.mockResolvedValue(undefined) prepareChat.mockResolvedValue({ execute: executeChat }) }) @@ -154,7 +152,7 @@ describe('requested cleanup stages', () => { it('records committed log deletion if attached storage cleanup fails', async () => { queueTableRows(schemaMock.workflowExecutionLogs, [{ id: 'one', files: [{ key: 'blob' }] }]) dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'one', files: [{ key: 'blob' }] }]) - storage.mockResolvedValue({ deleted: 0, failed: [{ key: 'blob', error: 'unavailable' }] }) + storage.mockRejectedValue(new Error('unavailable')) const run = control('workflowLogs', false) await expect(runBoundedLogScope(scope, run)).rejects.toThrow('storage cleanup is incomplete') expect(dbChainMockFns.delete).toHaveBeenCalledOnce() diff --git a/apps/sim/executor/handlers/variables/variables-handler.test.ts b/apps/sim/executor/handlers/variables/variables-handler.test.ts index aee2cc9e1dd..9bf06bf45a7 100644 --- a/apps/sim/executor/handlers/variables/variables-handler.test.ts +++ b/apps/sim/executor/handlers/variables/variables-handler.test.ts @@ -13,8 +13,7 @@ const { mockUploadFile } = vi.hoisted(() => ({ mockUploadFile: vi.fn(), })) -vi.mock('@/lib/execution/payloads/large-value-metadata', async (importOriginal) => ({ - ...(await importOriginal()), +vi.mock('@/lib/execution/payloads/large-value-metadata', () => ({ registerLargeValueOwner: vi.fn().mockResolvedValue(true), addLargeValueReference: vi.fn().mockResolvedValue(undefined), })) diff --git a/apps/sim/executor/orchestrators/loop.test.ts b/apps/sim/executor/orchestrators/loop.test.ts index 67b8ee15988..7f6dacb08d8 100644 --- a/apps/sim/executor/orchestrators/loop.test.ts +++ b/apps/sim/executor/orchestrators/loop.test.ts @@ -22,8 +22,7 @@ const mockLogger = vi.mocked(createLogger).mock.calls.findIndex(([name]) => name === 'LoopOrchestrator') ].value -vi.mock('@/lib/execution/payloads/large-value-metadata', async (importOriginal) => ({ - ...(await importOriginal()), +vi.mock('@/lib/execution/payloads/large-value-metadata', () => ({ registerLargeValueOwner: vi.fn().mockResolvedValue(true), addLargeValueReference: vi.fn().mockResolvedValue(undefined), })) diff --git a/apps/sim/executor/variables/resolvers/block.test.ts b/apps/sim/executor/variables/resolvers/block.test.ts index ef9b28102ec..75add9b4ada 100644 --- a/apps/sim/executor/variables/resolvers/block.test.ts +++ b/apps/sim/executor/variables/resolvers/block.test.ts @@ -7,8 +7,7 @@ import { navigatePathAsync } from '@/executor/variables/resolvers/reference-asyn import { BlockResolver } from './block' import { RESOLVED_EMPTY, type ResolutionContext } from './reference' -vi.mock('@/lib/execution/payloads/large-value-metadata', async (importOriginal) => ({ - ...(await importOriginal()), +vi.mock('@/lib/execution/payloads/large-value-metadata', () => ({ registerLargeValueOwner: vi.fn().mockResolvedValue(true), addLargeValueReference: vi.fn().mockResolvedValue(undefined), })) diff --git a/apps/sim/lib/cleanup/bounded-cleanup.md b/apps/sim/lib/cleanup/bounded-cleanup.md index 18de9bef090..f43fa5784cb 100644 --- a/apps/sim/lib/cleanup/bounded-cleanup.md +++ b/apps/sim/lib/cleanup/bounded-cleanup.md @@ -53,7 +53,7 @@ Limits count **selected roots**, including roots restored before deletion. They - Bounded dispatch sets one attempt and a 180-second hard maximum. The worker stops starting new root batches after 120 seconds; cancellable child preparation also observes that work deadline. - Cleanup SQL uses transaction-local **500ms lock_timeout** and **5s statement_timeout**. The billable-file delete and storage decrement remain atomic. Orphan knowledge-base storage cleanup reuses the binding lock held by document creation, with a 15-second storage deadline. Large-value reference writers lock the value before registering references; cleanup locks and rechecks it before claiming a tombstone. - Trigger `cleanup` metadata reports each requested type's `selected`, `deleted`, `skipped`, `filesDeleted`, and `filesFailed`, plus stage, duration, and stop reason: `budgets_exhausted`, `scopes_exhausted`, `time_budget`, or `failed`. -- A failure stops subsequent stages, preserves completed progress, and fails the run. External deletion is not transactional with Postgres. A hard process termination may leave the last metadata checkpoint behind actual effects. Do not blindly replay failed runs: inspect the failed stage and any external work already completed. Log and file storage is removed only for rows returned by the guarded delete. Their keys, and claimed large-value keys, enter `retention.storage.cleanup` outbox events in the same database transaction. The run attempts those events immediately; the existing outbox worker retries failures without selecting more roots. Inspect pending/dead-letter events when a run fails. Chat backend/storage cleanup still has its existing post-parent-delete recovery gap. +- A failure stops subsequent stages, preserves completed progress, and fails the run. External deletion is not transactional with Postgres. A hard process termination may leave the last metadata checkpoint behind actual effects. Do not blindly replay failed runs: inspect the failed stage and any external work already completed. Log and file storage is removed only for rows returned by the guarded delete. Their keys, and claimed large-value keys, enter `retention.storage.cleanup` outbox events in the same database transaction. Each event captures metadata IDs, content versions, and deletion state; retries skip changed or newly registered bindings and lock matching rows through a cancellable 15-second storage deletion. The run attempts those events immediately; the existing outbox worker retries failures without selecting more roots. Inspect pending/dead-letter events when a run fails. Chat backend/storage cleanup still has its existing post-parent-delete recovery gap. ## Rollout diff --git a/apps/sim/lib/cleanup/bounded.postgres.integration.ts b/apps/sim/lib/cleanup/bounded.postgres.integration.ts index da9be851786..e45a64a017a 100644 --- a/apps/sim/lib/cleanup/bounded.postgres.integration.ts +++ b/apps/sim/lib/cleanup/bounded.postgres.integration.ts @@ -13,8 +13,8 @@ import { import { processOutboxEventById } from '@/lib/core/outbox/service' import { lockLargeValueKeysForReference } from '@/lib/execution/payloads/large-value-lock' -const { deleteStorageFiles } = vi.hoisted(() => ({ deleteStorageFiles: vi.fn() })) -vi.mock('@/lib/uploads', () => ({ StorageService: { deleteFiles: deleteStorageFiles } })) +const { deleteStorageFile } = vi.hoisted(() => ({ deleteStorageFile: vi.fn() })) +vi.mock('@/lib/uploads', () => ({ StorageService: { deleteFile: deleteStorageFile } })) const url = new URL(process.env.DATABASE_URL ?? '') if (url.hostname !== '127.0.0.1' || url.pathname !== '/bounded_cleanup_test') { @@ -43,7 +43,7 @@ beforeAll(async () => { sql`CREATE TABLE execution_large_values (key text PRIMARY KEY, workspace_id text, deleted_at timestamp)` ) await db.execute( - sql`CREATE TABLE workspace_files (key text PRIMARY KEY, context text, deleted_at timestamp)` + sql`CREATE TABLE workspace_files (id text PRIMARY KEY DEFAULT gen_random_uuid()::text, key text NOT NULL, context text, deleted_at timestamp, content_updated_at timestamp NOT NULL DEFAULT now())` ) await db.execute(sql`CREATE TABLE workflow_execution_logs (execution_id text)`) await db.execute(sql`CREATE TABLE paused_executions (execution_id text, status text)`) @@ -56,7 +56,7 @@ beforeAll(async () => { ) }) beforeEach(async () => { - deleteStorageFiles.mockReset() + deleteStorageFile.mockReset() await db.execute(sql`TRUNCATE outbox_event`) await db.execute( sql`TRUNCATE execution_large_value_references, execution_large_value_dependencies, execution_large_values, workflow_execution_logs, paused_executions, workspace_files` @@ -269,7 +269,10 @@ describe('large-value reference lifecycle locks', () => { const table = kind === 'metadata' ? 'execution_large_values' : 'workspace_files' if (kind === 'metadata') await db.execute(sql`INSERT INTO execution_large_values VALUES ('live-key','one',NULL)`) - else await db.execute(sql`INSERT INTO workspace_files VALUES ('live-key','execution',NULL)`) + else + await db.execute( + sql`INSERT INTO workspace_files (key,context,deleted_at) VALUES ('live-key','execution',NULL)` + ) await db.transaction(async (tx) => { await lockLargeValueKeysForReference(tx, ['live-key']) await expect( @@ -310,43 +313,104 @@ describe('durable retention storage cleanup', () => { ).rejects.toThrow('abort transaction') expect(await count('roots')).toBe(6) expect((await db.execute(sql`SELECT id FROM outbox_event`)).length).toBe(0) - expect(deleteStorageFiles).not.toHaveBeenCalled() + expect(deleteStorageFile).not.toHaveBeenCalled() }) it('retries storage from the outbox after the root is gone', async () => { const [eventId] = await cleanupQuery(async (tx) => { await tx.delete(roots).where(eq(roots.id, 'a')) return enqueueRetentionStorageCleanup(tx, ['blob'], 'execution', 1) }) - deleteStorageFiles.mockResolvedValueOnce({ - deleted: 0, - failed: [{ key: 'blob', error: 'offline' }], - }) + deleteStorageFile.mockRejectedValueOnce(new Error('offline')) await expect( processRetentionStorageCleanup(control(1), 'workflows', [eventId]) ).rejects.toThrow('incomplete: pending') expect(await count('roots')).toBe(5) - const [pending] = await db.execute<{ status: string; payload: { keys: string[] } }>( - sql`SELECT status, payload FROM outbox_event WHERE id = ${eventId}` - ) + const [pending] = await db.execute<{ + status: string + payload: { files: Array<{ key: string }> } + }>(sql`SELECT status, payload FROM outbox_event WHERE id = ${eventId}`) expect(pending.status).toBe('pending') - expect(pending.payload.keys).toEqual(['blob']) + expect(pending.payload.files.map((file) => file.key)).toEqual(['blob']) await db.execute( sql`UPDATE outbox_event SET available_at = now() - interval '1 second' WHERE id = ${eventId}` ) - deleteStorageFiles.mockResolvedValueOnce({ deleted: 1, failed: [] }) + deleteStorageFile.mockResolvedValueOnce(undefined) await expect(processOutboxEventById(eventId, retentionStorageOutboxHandlers)).resolves.toBe( 'completed' ) - expect(deleteStorageFiles).toHaveBeenCalledTimes(2) + expect(deleteStorageFile).toHaveBeenCalledTimes(2) expect(await count('roots')).toBe(5) }) + it.each(['restore', 'replace', 'new-binding'] as const)( + 'does not delete a newer live binding on retry: %s', + async (change) => { + if (change !== 'new-binding') { + await db.execute(sql`INSERT INTO workspace_files(id,key,context,deleted_at) + VALUES ('original','blob','execution',now() - interval '1 day')`) + } + const [eventId] = await cleanupQuery((tx) => + enqueueRetentionStorageCleanup(tx, ['blob'], 'execution', 1, true) + ) + deleteStorageFile.mockRejectedValueOnce(new Error('offline')) + await expect(processOutboxEventById(eventId, retentionStorageOutboxHandlers)).resolves.toBe( + 'pending' + ) + if (change === 'restore') { + await db.execute(sql`UPDATE workspace_files SET deleted_at = NULL, + content_updated_at = content_updated_at + interval '1 second' WHERE id = 'original'`) + } else { + // A replacement may coexist with an older tombstone for the same key. + await db.execute( + sql`INSERT INTO workspace_files(id,key,context) VALUES ('replacement','blob','execution')` + ) + } + await db.execute( + sql`UPDATE outbox_event SET available_at = now() - interval '1 second' WHERE id = ${eventId}` + ) + await expect(processOutboxEventById(eventId, retentionStorageOutboxHandlers)).resolves.toBe( + 'completed' + ) + expect(deleteStorageFile).toHaveBeenCalledTimes(1) + const active = await db.execute( + sql`SELECT id FROM workspace_files WHERE key = 'blob' AND deleted_at IS NULL` + ) + expect(active).toHaveLength(1) + } + ) + it('holds the captured binding lock through storage deletion and tombstones only that identity', async () => { + await db.execute( + sql`INSERT INTO workspace_files(id,key,context) VALUES ('original','blob','execution')` + ) + const [eventId] = await cleanupQuery((tx) => + enqueueRetentionStorageCleanup(tx, ['blob'], 'execution', 1, true) + ) + deleteStorageFile.mockImplementationOnce(async () => { + await expect( + runOutsideTransactionContext(() => + db.transaction(async (tx) => { + await tx.execute(sql`SET LOCAL lock_timeout = '100ms'`) + await tx.execute( + sql`UPDATE workspace_files SET content_updated_at = now() WHERE id = 'original'` + ) + }) + ) + ).rejects.toThrow() + }) + await expect(processOutboxEventById(eventId, retentionStorageOutboxHandlers)).resolves.toBe( + 'completed' + ) + expect(deleteStorageFile).toHaveBeenCalledOnce() + expect( + await db.execute(sql`SELECT id FROM workspace_files WHERE deleted_at IS NULL`) + ).toHaveLength(0) + }) it('bounds and deduplicates the persisted key batches', async () => { await cleanupQuery((tx) => enqueueRetentionStorageCleanup(tx, ['a', 'b', 'a', 'c'], 'workspace', 2) ) - const events = await db.execute<{ payload: { keys: string[] } }>( + const events = await db.execute<{ payload: { files: Array<{ key: string }> } }>( sql`SELECT payload FROM outbox_event ORDER BY created_at, id` ) - expect(events.map((event) => event.payload.keys.length).sort()).toEqual([1, 2]) + expect(events.map((event) => event.payload.files.length).sort()).toEqual([1, 2]) }) }) diff --git a/apps/sim/lib/cleanup/bounded.ts b/apps/sim/lib/cleanup/bounded.ts index 532256f184b..48078e1c1a6 100644 --- a/apps/sim/lib/cleanup/bounded.ts +++ b/apps/sim/lib/cleanup/bounded.ts @@ -18,7 +18,7 @@ export async function setCleanupTimeouts(tx: Pick await tx.execute(sql`SET LOCAL statement_timeout = '5s'`) } -/** One short DB transaction; callers do storage/backend work outside this callback. */ +/** One bounded DB transaction; storage under a binding lock must use a cancellable deadline. */ export const cleanupQuery: CleanupQuery = (query) => dbFor('cleanup').transaction(async (tx) => { await setCleanupTimeouts(tx) diff --git a/apps/sim/lib/cleanup/storage-outbox.ts b/apps/sim/lib/cleanup/storage-outbox.ts index 8d096a3efc0..c0bb9864eea 100644 --- a/apps/sim/lib/cleanup/storage-outbox.ts +++ b/apps/sim/lib/cleanup/storage-outbox.ts @@ -1,6 +1,6 @@ import { workspaceFiles } from '@sim/db/schema' import { chunkArray } from '@sim/utils/helpers' -import { and, inArray, isNull } from 'drizzle-orm' +import { and, asc, eq, inArray } from 'drizzle-orm' import { z } from 'zod' import { type BoundedCleanup, type CleanupTransaction, cleanupQuery } from '@/lib/cleanup/bounded' import type { CleanupType } from '@/lib/cleanup/bounded-types' @@ -12,9 +12,18 @@ import { import type { StorageContext } from '@/lib/uploads/shared/types' const EVENT = 'retention.storage.cleanup' +const bindingSchema = z.object({ + id: z.string().min(1), + context: z.string().min(1), + contentUpdatedAt: z.iso.datetime(), + deletedAt: z.iso.datetime().nullable(), +}) const payloadSchema = z .object({ - keys: z.array(z.string().min(1)).min(1).max(500), + files: z + .array(z.object({ key: z.string().min(1), bindings: z.array(bindingSchema) })) + .min(1) + .max(500), context: z.enum([ 'knowledge-base', 'chat', @@ -33,37 +42,94 @@ const payloadSchema = z }) .strict() +const bindingColumns = { + id: workspaceFiles.id, + key: workspaceFiles.key, + context: workspaceFiles.context, + contentUpdatedAt: workspaceFiles.contentUpdatedAt, + deletedAt: workspaceFiles.deletedAt, +} +function identity(row: { + id: string + context: string + contentUpdatedAt: Date + deletedAt: Date | null +}): z.infer { + return { + id: row.id, + context: row.context, + contentUpdatedAt: row.contentUpdatedAt.toISOString(), + deletedAt: row.deletedAt?.toISOString() ?? null, + } +} + function handler(onResult?: (deleted: number, failed: number) => Promise): OutboxHandler { return async (rawPayload, context) => { const payload = payloadSchema.parse(rawPayload) - context.signal.throwIfAborted() const { StorageService } = await import('@/lib/uploads') - let result: Awaited> - try { - result = await StorageService.deleteFiles(payload.keys, payload.context) - } catch (error) { - await onResult?.(0, payload.keys.length) - throw error - } - await onResult?.(result.deleted, result.failed.length) - if (result.failed.length > 0) throw new Error('Retention storage deletions failed') - context.signal.throwIfAborted() - if (payload.tombstoneFiles) { - await cleanupQuery(async (tx) => { - await tx - .update(workspaceFiles) - .set({ deletedAt: new Date() }) - .where(and(inArray(workspaceFiles.key, payload.keys), isNull(workspaceFiles.deletedAt))) - }) + for (const file of payload.files) { + context.signal.throwIfAborted() + let deleted: boolean + try { + deleted = await cleanupQuery(async (tx) => { + // The lock coordinates restoration of this generation with the bounded storage call. + const current = await tx + .select(bindingColumns) + .from(workspaceFiles) + .where(eq(workspaceFiles.key, file.key)) + .orderBy(asc(workspaceFiles.id)) + .for('update') + const expected = new Map(file.bindings.map((binding) => [binding.id, binding])) + if ( + current.length !== expected.size || + current.some((row) => { + const binding = expected.get(row.id) + const actual = identity(row) + return ( + !binding || + actual.context !== binding.context || + actual.context !== payload.context || + actual.contentUpdatedAt !== binding.contentUpdatedAt || + actual.deletedAt !== binding.deletedAt || + (!payload.tombstoneFiles && actual.deletedAt === null) + ) + }) + ) + return false + + const signal = AbortSignal.any([context.signal, AbortSignal.timeout(15_000)]) + await StorageService.deleteFile({ key: file.key, context: payload.context, signal }) + signal.throwIfAborted() + if (payload.tombstoneFiles && current.length) { + await tx + .update(workspaceFiles) + .set({ deletedAt: new Date() }) + .where( + and( + eq(workspaceFiles.key, file.key), + inArray( + workspaceFiles.id, + current.map((row) => row.id) + ) + ) + ) + } + return true + }) + } catch (error) { + await onResult?.(0, 1) + throw error + } + if (deleted) await onResult?.(1, 0) } } } export const retentionStorageOutboxHandlers = { [EVENT]: handler() } -/** Persist immutable keys in the same transaction that deletes or claims their roots. */ +/** Snapshot metadata generations in the transaction that deletes or claims their roots. */ export async function enqueueRetentionStorageCleanup( - tx: Pick, + tx: Pick, keys: string[], context: StorageContext, batchSize: number, @@ -71,7 +137,17 @@ export async function enqueueRetentionStorageCleanup( ): Promise { const events: string[] = [] for (const batch of chunkArray([...new Set(keys)], batchSize)) { - const payload = payloadSchema.parse({ keys: batch, context, tombstoneFiles }) + const bindings = await tx + .select(bindingColumns) + .from(workspaceFiles) + .where(inArray(workspaceFiles.key, batch)) + .orderBy(asc(workspaceFiles.id)) + .for('update') + const files = batch.map((key) => ({ + key, + bindings: bindings.filter((row) => row.key === key).map(identity), + })) + const payload = payloadSchema.parse({ files, context, tombstoneFiles }) events.push(await enqueueOutboxEvent(tx, EVENT, payload, { maxAttempts: 48 })) } return events diff --git a/apps/sim/lib/function-execution/execute-request.test.ts b/apps/sim/lib/function-execution/execute-request.test.ts index e1d307f91d2..3d46307bfa5 100644 --- a/apps/sim/lib/function-execution/execute-request.test.ts +++ b/apps/sim/lib/function-execution/execute-request.test.ts @@ -71,8 +71,7 @@ const { mockRenderedMountContributors: vi.fn(), })) -vi.mock('@/lib/execution/payloads/large-value-metadata', async (importOriginal) => ({ - ...(await importOriginal()), +vi.mock('@/lib/execution/payloads/large-value-metadata', () => ({ registerLargeValueOwner: vi.fn().mockResolvedValue(true), addLargeValueReference: vi.fn().mockResolvedValue(undefined), })) diff --git a/apps/sim/lib/uploads/utils/user-file-base64.server.test.ts b/apps/sim/lib/uploads/utils/user-file-base64.server.test.ts index 64f0ad7db82..181cbfae667 100644 --- a/apps/sim/lib/uploads/utils/user-file-base64.server.test.ts +++ b/apps/sim/lib/uploads/utils/user-file-base64.server.test.ts @@ -41,8 +41,7 @@ const mockGetRedisClient = redisConfigMockFns.mockGetRedisClient afterAll(resetRedisConfigMock) -vi.mock('@/lib/execution/payloads/large-value-metadata', async (importOriginal) => ({ - ...(await importOriginal()), +vi.mock('@/lib/execution/payloads/large-value-metadata', () => ({ registerLargeValueOwner: vi.fn().mockResolvedValue(true), addLargeValueReference: vi.fn().mockResolvedValue(undefined), })) diff --git a/apps/sim/lib/workflows/streaming/streaming.test.ts b/apps/sim/lib/workflows/streaming/streaming.test.ts index c7e028eb0b9..8adea118e0d 100644 --- a/apps/sim/lib/workflows/streaming/streaming.test.ts +++ b/apps/sim/lib/workflows/streaming/streaming.test.ts @@ -30,8 +30,7 @@ const { mockDownloadFile } = vi.hoisted(() => ({ mockDownloadFile: vi.fn(), })) -vi.mock('@/lib/execution/payloads/large-value-metadata', async (importOriginal) => ({ - ...(await importOriginal()), +vi.mock('@/lib/execution/payloads/large-value-metadata', () => ({ registerLargeValueOwner: vi.fn().mockResolvedValue(true), addLargeValueReference: vi.fn().mockResolvedValue(undefined), }))