Skip to content

Commit 7c68116

Browse files
committed
fix: preserve committed write results and bound retention reads
1 parent dded5e8 commit 7c68116

4 files changed

Lines changed: 59 additions & 40 deletions

File tree

‎apps/sim/lib/file-retention/folders.ts‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,17 +53,6 @@ export async function cleanupExpiredFileFolderInTx(
5353
.where(and(folderScope, eq(folder.id, child.id)))
5454
}
5555
}
56-
const rootFiles = await tx
57-
.select({ name: workspaceFiles.originalName })
58-
.from(workspaceFiles)
59-
.where(
60-
and(
61-
fileOwnerCondition(owner),
62-
workspaceFileNameFolderCondition(null),
63-
isNull(workspaceFiles.deletedAt)
64-
)
65-
)
66-
const rootNames = new Set(rootFiles.map(({ name }) => name))
6756
for (;;) {
6857
const children = await tx
6958
.select({ id: workspaceFiles.id, name: workspaceFiles.originalName })
@@ -79,11 +68,40 @@ export async function cleanupExpiredFileFolderInTx(
7968
.limit(FILES_PER_QUERY)
8069
.for('update')
8170
if (!children.length) break
71+
const candidateNames = new Set(children.map(({ name }) => name))
72+
const rootFiles = await tx
73+
.select({ name: workspaceFiles.originalName })
74+
.from(workspaceFiles)
75+
.where(
76+
and(
77+
fileOwnerCondition(owner),
78+
workspaceFileNameFolderCondition(null),
79+
isNull(workspaceFiles.deletedAt),
80+
inArray(workspaceFiles.originalName, [...candidateNames])
81+
)
82+
)
83+
const rootNames = new Set(rootFiles.map(({ name }) => name))
8284
const renamed: { id: string; name: string }[] = []
8385
for (const child of children) {
8486
const name = await generateRestoreName(
8587
child.name,
86-
async (candidate) => rootNames.has(candidate),
88+
async (candidate) => {
89+
if (rootNames.has(candidate)) return true
90+
if (candidateNames.has(candidate)) return false
91+
const [existing] = await tx
92+
.select({ id: workspaceFiles.id })
93+
.from(workspaceFiles)
94+
.where(
95+
and(
96+
fileOwnerCondition(owner),
97+
workspaceFileNameFolderCondition(null),
98+
isNull(workspaceFiles.deletedAt),
99+
eq(workspaceFiles.originalName, candidate)
100+
)
101+
)
102+
.limit(1)
103+
return Boolean(existing)
104+
},
87105
{ hasExtension: true }
88106
)
89107
rootNames.add(name)

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2705,8 +2705,7 @@ export async function updateWorkspaceFileContent(
27052705
}
27062706
await finishFileContentEffects(
27072707
{ cleanupIds: finalized.storageCleanupEventIds, liveDocEventId: finalized.liveDocEventId },
2708-
{ workspaceId, fileId, reason: 'released version' },
2709-
'defer'
2708+
{ workspaceId, fileId, reason: 'released version' }
27102709
)
27112710

27122711
const currentFolderPath =

‎apps/sim/lib/uploads/server/content-effects.test.ts‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { createLogger } from '@sim/logger'
12
import { beforeEach, describe, expect, it, vi } from 'vitest'
23

34
const mocks = vi.hoisted(() => ({ cleanup: vi.fn(), reconcile: vi.fn() }))
@@ -22,12 +23,23 @@ describe('committed file reconciliation policy', () => {
2223
await expect(finishFileContentEffects(effects, {})).resolves.toBeUndefined()
2324
}
2425
)
25-
it.each(['dead_letter', 'not_found'])('propagates a terminal %s result', async (status) => {
26-
mocks.reconcile.mockResolvedValue(status)
27-
await expect(finishFileContentEffects(effects, {})).rejects.toThrow(status)
28-
})
29-
it('retains the explicit defer policy for a terminal result', async () => {
30-
mocks.reconcile.mockResolvedValue('dead_letter')
31-
await expect(finishFileContentEffects(effects, {}, 'defer')).resolves.toBeUndefined()
26+
it.each(['dead_letter', 'not_found'])(
27+
'reports a terminal %s result without failing the committed write',
28+
async (status) => {
29+
mocks.reconcile.mockResolvedValue(status)
30+
await expect(finishFileContentEffects(effects, {})).resolves.toBeUndefined()
31+
expect(createLogger('FileContentEffects').error).toHaveBeenCalledWith(
32+
'Committed file live-document reconciliation requires intervention',
33+
expect.objectContaining({ eventId: 'event-1', result: status })
34+
)
35+
}
36+
)
37+
it('reports an inline processing exception without failing the committed write', async () => {
38+
mocks.reconcile.mockRejectedValue(new Error('database unavailable'))
39+
await expect(finishFileContentEffects(effects, {})).resolves.toBeUndefined()
40+
expect(createLogger('FileContentEffects').error).toHaveBeenCalledWith(
41+
'Committed file live-document reconciliation failed inline',
42+
expect.objectContaining({ eventId: 'event-1' })
43+
)
3244
})
3345
})

‎apps/sim/lib/uploads/server/content-effects.ts‎

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,36 +5,26 @@ import { processFileLiveDocReconciliationNow } from '@/lib/uploads/server/live-d
55

66
const logger = createLogger('FileContentEffects')
77

8-
/** Processes durable content effects after commit, retaining the caller's inline failure policy. */
8+
/** Reports durable effect failures without changing the outcome of an already committed write. */
99
export async function finishFileContentEffects(
1010
effects: { cleanupIds: readonly string[]; liveDocEventId?: string },
11-
logContext: Record<string, unknown>,
12-
reconciliationFailure: 'propagate' | 'defer' = 'propagate'
11+
logContext: Record<string, unknown>
1312
): Promise<void> {
1413
await processWorkspaceFileStorageCleanupsNow(effects.cleanupIds, logContext)
1514
if (!effects.liveDocEventId) return
16-
if (reconciliationFailure === 'propagate') {
15+
const context = { ...logContext, eventId: effects.liveDocEventId }
16+
try {
1717
const result = await processFileLiveDocReconciliationNow(effects.liveDocEventId)
1818
if (result === 'dead_letter' || result === 'not_found') {
19-
throw new Error(`Live document reconciliation could not complete: ${result}`)
20-
}
21-
if (result !== 'completed') {
22-
logger.warn('Live document reconciliation remains pending', {
23-
...logContext,
24-
eventId: effects.liveDocEventId,
19+
logger.error('Committed file live-document reconciliation requires intervention', {
20+
...context,
2521
result,
2622
})
27-
}
28-
return
29-
}
30-
const context = { ...logContext, eventId: effects.liveDocEventId }
31-
try {
32-
const result = await processFileLiveDocReconciliationNow(effects.liveDocEventId)
33-
if (result !== 'completed') {
34-
logger.warn('Live document reconciliation deferred to outbox retry', { ...context, result })
23+
} else if (result !== 'completed') {
24+
logger.warn('Live document reconciliation remains pending', { ...context, result })
3525
}
3626
} catch (error) {
37-
logger.warn('Live document reconciliation deferred after inline processing error', {
27+
logger.error('Committed file live-document reconciliation failed inline', {
3828
...context,
3929
error: describeError(error),
4030
})

0 commit comments

Comments
 (0)