diff --git a/apps/sim/app/api/files/uploads/finalizers.test.ts b/apps/sim/app/api/files/uploads/finalizers.test.ts index 37434dbac84..4483401fd18 100644 --- a/apps/sim/app/api/files/uploads/finalizers.test.ts +++ b/apps/sim/app/api/files/uploads/finalizers.test.ts @@ -69,7 +69,11 @@ vi.mock('@/lib/users/queries', () => ({ requireResolvedUserEmail: (emails: Map, userId: string) => emails.get(userId)!, })) -import { finalizeUploadPurpose } from '@/app/api/files/uploads/finalizers' +import { + finalizeUploadPurpose, + loadCompletedUploadPurpose, +} from '@/app/api/files/uploads/finalizers' +import type { InternalUploadPurpose } from '@/app/api/files/uploads/purposes' const now = new Date('2026-08-04T12:00:00.000Z') const actor = { id: 'user-1', name: 'Ada', email: 'ada@example.com' } @@ -138,6 +142,70 @@ const workspaceFile = { updatedAt: now, } +/** + * How each purpose replays a completion. `Record` over the union is a + * compile-time completeness gate: adding a purpose fails to build until its + * replay behavior is declared here. + */ +const REPLAY_ROUTE: Record = { + workspace_file: 'loader', + profile_picture: 'idempotent-finalizer', + workspace_logo: 'idempotent-finalizer', + mothership_attachment: 'idempotent-finalizer', + execution_attachment: 'idempotent-finalizer', +} + +const purposesReplayedBy = (route: 'loader' | 'idempotent-finalizer') => + (Object.keys(REPLAY_ROUTE) as InternalUploadPurpose[]).filter((p) => REPLAY_ROUTE[p] === route) + +describe('completion replay contract', () => { + const REPLAY_VIA_IDEMPOTENT_FINALIZER = purposesReplayedBy('idempotent-finalizer') + + beforeEach(() => { + vi.clearAllMocks() + }) + + it.each(REPLAY_VIA_IDEMPOTENT_FINALIZER)( + 'does not mark %s as loader-backed, so its replay re-runs the idempotent finalizer', + async (purpose) => { + mockSelectLimit.mockResolvedValue([]) + mockInsertReturning.mockResolvedValue([metadataRow]) + const request = new NextRequest('http://localhost/api/files/uploads/upload-1/complete') + + const finalized = await finalizeUploadPurpose({ + session: { ...uploadSession, purpose }, + actor, + principal, + request, + }) + + expect(finalized.completedFileId).toBeUndefined() + } + ) + + it.each(REPLAY_VIA_IDEMPOTENT_FINALIZER)( + 'reports a classified error rather than an unhandled crash if %s ever reaches the loader', + async (purpose) => { + await expect(loadCompletedUploadPurpose({ ...uploadSession, purpose })).rejects.toMatchObject( + { code: 'internal' } + ) + } + ) + + it('loads workspace_file from its durable record on replay', async () => { + mockGetWorkspaceFile.mockResolvedValueOnce(workspaceFile) + + const loaded = await loadCompletedUploadPurpose({ + ...uploadSession, + purpose: purposesReplayedBy('loader')[0], + completedFileId: workspaceFile.id, + }) + + expect(loaded).toMatchObject({ id: workspaceFile.id }) + expect(mockGetWorkspaceFile).toHaveBeenCalledTimes(1) + }) +}) + describe('upload purpose finalizers', () => { beforeEach(() => { vi.clearAllMocks() diff --git a/apps/sim/app/api/files/uploads/finalizers.ts b/apps/sim/app/api/files/uploads/finalizers.ts index a59eec3beb0..13d16c1df5b 100644 --- a/apps/sim/app/api/files/uploads/finalizers.ts +++ b/apps/sim/app/api/files/uploads/finalizers.ts @@ -59,6 +59,14 @@ interface FinalizeUploadPurposeParams { interface FinalizedUploadPurpose { value: UploadPurposeResult + /** + * Recorded on the session so a replayed completion returns the original + * result instead of re-running a finalizer with one-time side effects. + * + * Set only for a purpose {@link loadCompletedUploadPurpose} can reload; an + * already-idempotent finalizer must leave it undefined so replays flow back + * through the finalizer itself. + */ completedFileId?: string } @@ -111,13 +119,30 @@ export async function finalizeUploadPurpose({ } } +/** + * Reloads the durable result of an already-completed session, for the purposes + * that report a {@link FinalizedUploadPurpose.completedFileId}. + * + * The switch is exhaustive so that adding a purpose is a compile error until + * its replay behavior is decided here. + */ export async function loadCompletedUploadPurpose( session: UploadSessionRecord ): Promise { - if (session.purpose !== 'workspace_file') { - throw new Error(`Upload purpose ${session.purpose} has no durable file result`) + switch (session.purpose) { + case 'workspace_file': + return toV2File(await loadCompletedWorkspaceFileUpload(session)) + case 'profile_picture': + case 'workspace_logo': + case 'mothership_attachment': + case 'execution_attachment': + case 'table_import': + case 'knowledge_document': + throw new UploadSessionError( + 'internal', + `Upload purpose ${session.purpose} recorded a completed file but has no durable loader` + ) } - return toV2File(await loadCompletedWorkspaceFileUpload(session)) } async function finalizeInternalWorkspaceFile( @@ -314,7 +339,6 @@ async function finalizeExecutionAttachment( key: session.storageKey, context: 'execution', }, - completedFileId: finalized.file.id, } } diff --git a/apps/sim/lib/uploads/upload-session/service.ts b/apps/sim/lib/uploads/upload-session/service.ts index 240bbdd7981..a4386de8753 100644 --- a/apps/sim/lib/uploads/upload-session/service.ts +++ b/apps/sim/lib/uploads/upload-session/service.ts @@ -701,6 +701,13 @@ export async function completeUploadSession(params: { } } +/** + * The completion marker is only written when the finalizer reports one. A + * finalizer that instead records it inside its own registration transaction — + * see `markUploadSessionFileRegistered` — keeps that value: clearing it would + * let the abort guard and the expiry sweep treat a session whose durable + * resource already exists as disposable. + */ async function markUploadSessionCompleted( session: UploadSessionRecord, leaseId: string, @@ -711,7 +718,7 @@ async function markUploadSessionCompleted( .update(uploadSession) .set({ status: 'completed', - completedFileId, + ...(completedFileId !== null ? { completedFileId } : {}), completedAt, processingLeaseId: null, processingLeaseExpiresAt: null,