Skip to content

Commit 2c65f4b

Browse files
committed
fix(files): release purged history inside the file-row purge transaction
1 parent cc2cbc0 commit 2c65f4b

9 files changed

Lines changed: 108 additions & 108 deletions

File tree

‎apps/docs/openapi-v2-files-audit.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4569,7 +4569,7 @@
45694569
"source": {
45704570
"type": "string",
45714571
"enum": ["upload", "user", "api", "copilot", "workflow", "collab", "revert", "unknown"],
4572-
"description": "What wrote this version: `upload` (the original upload), `user` (a save in the Sim editor), `api` (an API, CLI, or MCP write), `copilot` (Sim, the agent), `workflow` (a workflow run), `collab` (collaborative editing), `revert` (a revert to an earlier version), or `unknown` (content written before version history existed)."
4572+
"description": "What wrote this version: `upload` (the original upload), `user` (a save in the Sim editor), `api` (an API, CLI, or MCP write), `copilot` (Sim, the agent), `workflow` (a workflow run), `collab` (collaborative editing), `revert` (a revert to an earlier version), or `unknown` (content written before version history existed, or by a writer with no source of its own)."
45734573
},
45744574
"authors": {
45754575
"type": "array",

‎apps/sim/background/cleanup-file-versions.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,7 @@ import type { PlanCategory } from '@/lib/billing/plan-helpers'
99
import { DEFAULT_DELETE_CHUNK_SIZE } from '@/lib/cleanup/batch-delete'
1010
import { retentionCleanupQueue } from '@/lib/cleanup/queue'
1111
import { enqueueWorkspaceFileStorageCleanups } from '@/lib/uploads/contexts/workspace/workspace-file-storage-cleanup-outbox'
12-
import {
13-
FILE_VERSION_RETENTION_KEEP_LATEST,
14-
MAX_SUPERSEDED_FILE_VERSIONS,
15-
} from '@/lib/uploads/contexts/workspace/workspace-file-versions'
12+
import { MAX_SUPERSEDED_FILE_VERSIONS } from '@/lib/uploads/contexts/workspace/workspace-file-versions'
1613

1714
const logger = createLogger('CleanupFileVersions')
1815

@@ -39,6 +36,9 @@ const MAX_SUPERSEDED_VERSIONS_BY_PLAN: Record<PlanCategory, number> = {
3936
enterprise: MAX_SUPERSEDED_FILE_VERSIONS,
4037
}
4138

39+
/** Retention never prunes the newest versions of a file, whatever their age. */
40+
const FILE_VERSION_RETENTION_KEEP_LATEST = 10
41+
4242
/** Newest superseded versions a file keeps whatever their age (the current version is the tenth). */
4343
const KEEP_SUPERSEDED = FILE_VERSION_RETENTION_KEEP_LATEST - 1
4444

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

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,11 @@ vi.mock('@/lib/uploads', () => ({
8282

8383
vi.mock('@/lib/uploads/server/metadata', () => ({ deleteFileMetadata: mockDeleteFileMetadata }))
8484

85-
const { mockReleaseExpiredWorkspaceFileVersions } = vi.hoisted(() => ({
86-
mockReleaseExpiredWorkspaceFileVersions: vi.fn(),
85+
const { mockReleaseWorkspaceFileVersionsForPurgeInTx } = vi.hoisted(() => ({
86+
mockReleaseWorkspaceFileVersionsForPurgeInTx: vi.fn(),
8787
}))
8888
vi.mock('@/lib/uploads/contexts/workspace/workspace-file-versions', () => ({
89-
releaseExpiredWorkspaceFileVersions: mockReleaseExpiredWorkspaceFileVersions,
89+
releaseWorkspaceFileVersionsForPurgeInTx: mockReleaseWorkspaceFileVersionsForPurgeInTx,
9090
}))
9191

9292
vi.mock('@/lib/workflows/utils', () => ({
@@ -129,7 +129,7 @@ describe('cleanup soft deletes', () => {
129129
})
130130
})
131131

132-
it('releases version history only for files whose current object was deleted', async () => {
132+
it('releases version history inside the transaction that purges the file rows', async () => {
133133
mockSelectRowsByIdChunks
134134
.mockResolvedValueOnce([])
135135
.mockResolvedValueOnce([])
@@ -148,28 +148,26 @@ describe('cleanup soft deletes', () => {
148148
context: 'workspace',
149149
sizeBytes: 4,
150150
},
151-
{
152-
id: 'chat-file',
153-
key: 'mothership/chat-file',
154-
workspaceId: 'ws-1',
155-
context: 'mothership',
156-
sizeBytes: 3,
157-
},
158151
])
159152
mockDeleteFiles.mockResolvedValueOnce({
160153
deleted: 1,
161154
failed: [{ key: 'workspace/ws-1/file-failed', error: 'storage unavailable' }],
162155
})
156+
dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'file-purged', sizeBytes: 5 }])
163157

164158
await runCleanupSoftDeletes(basePayload)
165159

166-
expect(mockReleaseExpiredWorkspaceFileVersions).toHaveBeenCalledWith(
167-
expect.anything(),
160+
expect(mockReleaseWorkspaceFileVersionsForPurgeInTx).toHaveBeenCalledOnce()
161+
expect(mockReleaseWorkspaceFileVersionsForPurgeInTx).toHaveBeenCalledWith(
162+
dbChainMock.db,
168163
['file-purged'],
169164
expect.any(Date)
170165
)
171-
expect(mockDeleteFiles.mock.invocationCallOrder[0]).toBeLessThan(
172-
mockReleaseExpiredWorkspaceFileVersions.mock.invocationCallOrder[0]
166+
expect(dbChainMockFns.transaction.mock.invocationCallOrder[0]).toBeLessThan(
167+
mockReleaseWorkspaceFileVersionsForPurgeInTx.mock.invocationCallOrder[0]
168+
)
169+
expect(mockReleaseWorkspaceFileVersionsForPurgeInTx.mock.invocationCallOrder[0]).toBeLessThan(
170+
dbChainMockFns.delete.mock.invocationCallOrder[0]
173171
)
174172
})
175173

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ import { hardDeleteDocuments } from '@/lib/knowledge/documents/service'
4444
import type { StorageContext } from '@/lib/uploads'
4545
import { isUsingCloudStorage, StorageService } from '@/lib/uploads'
4646
import { allocateUniqueWorkspaceFileName } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
47-
import { releaseExpiredWorkspaceFileVersions } from '@/lib/uploads/contexts/workspace/workspace-file-versions'
47+
import { releaseWorkspaceFileVersionsForPurgeInTx } from '@/lib/uploads/contexts/workspace/workspace-file-versions'
4848
import { deleteFileMetadata } from '@/lib/uploads/server/metadata'
4949
import { getWorkspaceFileSize } from '@/lib/uploads/shared/types'
5050
import { deduplicateWorkflowName } from '@/lib/workflows/utils'
@@ -333,6 +333,11 @@ async function deleteExpiredBillableWorkspaceFileRows(
333333
for (const batch of chunkArray(workspaceRows, DEFAULT_DELETE_CHUNK_SIZE)) {
334334
try {
335335
const deletedCount = await db.transaction(async (tx) => {
336+
await releaseWorkspaceFileVersionsForPurgeInTx(
337+
tx,
338+
batch.map(({ id }) => id),
339+
retentionDate
340+
)
336341
const deletedRows = await tx
337342
.delete(workspaceFiles)
338343
.where(
@@ -901,15 +906,6 @@ export async function runCleanupSoftDeletes(
901906
}
902907

903908
const fileCleanup = await cleanupWorkspaceFileStorage(fileScope)
904-
/**
905-
* Only files whose current object is gone are committed to the purge, so a file whose object
906-
* deletion failed keeps its history for the retry instead of losing it ahead of its own delete.
907-
*/
908-
await releaseExpiredWorkspaceFileVersions(
909-
cleanupDb,
910-
fileCleanup.multiContextRows.filter((row) => row.context === 'workspace').map((row) => row.id),
911-
retentionDate
912-
)
913909
if (budgets && fileCleanup.filesFailed) throw new Error('File storage cleanup failed')
914910

915911
let totalDeleted = 0

‎apps/sim/lib/api/contracts/v2/file-versions.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,16 @@ import {
2222

2323
/**
2424
* v2 file version history contracts. Every content write that changes the bytes — an upload, an
25-
* API or editor save, a Sim edit, a workflow write, a revert — records a version; collaborative
26-
* edits and repeated workflow writes from one author fold into one version per ten-minute window. Renames and moves
27-
* are metadata changes and never create versions, so every version reads under the file's
28-
* current name.
25+
* API or editor save, a Sim edit, a workflow write, a revert — records a version. Collaborative
26+
* edits and repeated workflow writes from one author fold into the current version until it is ten
27+
* minutes old or its writer has been idle for five. Renames and moves are metadata changes and never
28+
* create versions, so every version reads under the file's current name.
2929
*/
3030

3131
export const v2FileVersionSourceSchema = z
3232
.enum(['upload', 'user', 'api', 'copilot', 'workflow', 'collab', 'revert', 'unknown'])
3333
.describe(
34-
'What wrote this version: `upload` (the original upload), `user` (a save in the Sim editor), `api` (an API, CLI, or MCP write), `copilot` (Sim, the agent), `workflow` (a workflow run), `collab` (collaborative editing), `revert` (a revert to an earlier version), or `unknown` (content written before version history existed).'
34+
'What wrote this version: `upload` (the original upload), `user` (a save in the Sim editor), `api` (an API, CLI, or MCP write), `copilot` (Sim, the agent), `workflow` (a workflow run), `collab` (collaborative editing), `revert` (a revert to an earlier version), or `unknown` (content written before version history existed, or by a writer with no source of its own).'
3535
)
3636

3737
export const v2FileVersionAuthorSchema = z

‎apps/sim/lib/uploads/contexts/workspace/__integration__/file-versions.integration.ts‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import { WORKSPACE_FILE_STORAGE_CLEANUP_OUTBOX_EVENT } from '@/lib/uploads/conte
3939
import {
4040
getCurrentWorkspaceFileVersion,
4141
queryWorkspaceFileVersions,
42-
releaseExpiredWorkspaceFileVersions,
42+
releaseWorkspaceFileVersionsForPurgeInTx,
4343
} from '@/lib/uploads/contexts/workspace/workspace-file-versions'
4444
import { revertWorkspaceFileVersion } from '@/lib/workspace-files/application/file-versions'
4545
import { runCleanupFileVersions } from '@/background/cleanup-file-versions'
@@ -402,7 +402,7 @@ describe('workspace file version history in PostgreSQL', () => {
402402
).rejects.toMatchObject({ code: 'conflict' })
403403
})
404404

405-
it('releases an expired file history atomically and leaves a restored file untouched', async () => {
405+
it('releases history only with the purge transaction and leaves a restored file untouched', async () => {
406406
const expired = await seedFile('one')
407407
const restored = await seedFile('one')
408408
for (const fixture of [expired, restored]) {
@@ -423,10 +423,17 @@ describe('workspace file version history in PostgreSQL', () => {
423423
.filter((row) => row.supersededAt !== null)
424424
.map((row) => row.key)
425425

426-
await releaseExpiredWorkspaceFileVersions(
427-
db,
428-
[expired.fileId, restored.fileId],
429-
new Date(Date.now() - 30 * 24 * 60 * 60 * 1000)
426+
const cutoff = new Date(Date.now() - 30 * 24 * 60 * 60 * 1000)
427+
await expect(
428+
db.transaction(async (tx) => {
429+
await releaseWorkspaceFileVersionsForPurgeInTx(tx, [expired.fileId], cutoff)
430+
throw new Error('purge failed')
431+
})
432+
).rejects.toThrow('purge failed')
433+
expect((await versionRows(expired.fileId)).map((row) => row.version)).toEqual([1, 2, 3])
434+
435+
await db.transaction((tx) =>
436+
releaseWorkspaceFileVersionsForPurgeInTx(tx, [expired.fileId, restored.fileId], cutoff)
430437
)
431438

432439
expect((await versionRows(expired.fileId)).map((row) => row.version)).toEqual([3])

‎apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ describe('workspace file metadata and storage accounting', () => {
210210
mockProcessWorkspaceFileLiveDocReconciliationNow.mockResolvedValue('completed')
211211
mockReplaceWorkspaceFileSecretProvenanceInTx.mockResolvedValue(undefined)
212212
mockSaveCollabDocStateInTx.mockResolvedValue(undefined)
213-
mockRecordWorkspaceFileVersionInTx.mockResolvedValue({ releasedKeys: [] })
213+
mockRecordWorkspaceFileVersionInTx.mockResolvedValue({ version: 2, releasedKeys: [] })
214214
})
215215

216216
it('returns the canonical inserted record with the pre-resolved folder path', async () => {
@@ -785,7 +785,10 @@ describe('workspace file metadata and storage accounting', () => {
785785
dbChainMockFns.limit.mockResolvedValueOnce([FILE_ROW]).mockResolvedValueOnce([FILE_ROW])
786786
dbChainMockFns.returning.mockResolvedValueOnce([updatedFile])
787787
mockUploadFile.mockResolvedValueOnce({ key: replacementKey })
788-
mockRecordWorkspaceFileVersionInTx.mockResolvedValueOnce({ releasedKeys: [FILE_ROW.key] })
788+
mockRecordWorkspaceFileVersionInTx.mockResolvedValueOnce({
789+
version: 2,
790+
releasedKeys: [FILE_ROW.key],
791+
})
789792

790793
await updateWorkspaceFileContent(
791794
FILE_ROW.workspaceId,

0 commit comments

Comments
 (0)