fix(uploads): prevent multipart cleanup races

Author: TheodoreSpeaks

apps/sim/app/api/knowledge/[id]/documents/uploads/route.test.ts

@@ -5,27 +5,25 @@ import { NextRequest, NextResponse } from 'next/server'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 const {
-  mockCreateUploadSession,
+  mockCreateKnowledgeDocumentUploadSession,
   mockRequireKnowledgeDocumentUploadAccess,
   mockRequireKnowledgeDocumentUploadActor,
   mockRequireKnowledgeDocumentUploadBilling,
 } = vi.hoisted(() => ({
-  mockCreateUploadSession: vi.fn(),
+  mockCreateKnowledgeDocumentUploadSession: vi.fn(),
   mockRequireKnowledgeDocumentUploadAccess: vi.fn(),
   mockRequireKnowledgeDocumentUploadActor: vi.fn(),
   mockRequireKnowledgeDocumentUploadBilling: vi.fn(),
 }))
 
-vi.mock('@/lib/uploads/multipart-session/service', () => ({
-  createUploadSession: mockCreateUploadSession,
-}))
 vi.mock('@/app/api/knowledge/[id]/documents/uploads/utils', () => ({
   requireKnowledgeDocumentUploadAccess: mockRequireKnowledgeDocumentUploadAccess,
   requireKnowledgeDocumentUploadActor: mockRequireKnowledgeDocumentUploadActor,
   requireKnowledgeDocumentUploadBilling: mockRequireKnowledgeDocumentUploadBilling,
 }))
 vi.mock('@/app/api/files/uploads/utils', () => ({ uploadSessionErrorResponse: vi.fn() }))
 vi.mock('@/app/api/v2/knowledge/[id]/documents/uploads/utils', () => ({
+  createKnowledgeDocumentUploadSession: mockCreateKnowledgeDocumentUploadSession,
   toV2KnowledgeDocumentUpload: (session: Record<string, unknown>) => ({
     ...session,
     name: session.fileName,
@@ -66,7 +64,7 @@ describe('POST /api/knowledge/[id]/documents/uploads', () => {
       knowledgeBase: { id: 'kb-1', name: 'Docs', workspaceId: WORKSPACE_ID },
     })
     mockRequireKnowledgeDocumentUploadBilling.mockResolvedValue({ actorUserId: 'user-1' })
-    mockCreateUploadSession.mockResolvedValue({
+    mockCreateKnowledgeDocumentUploadSession.mockResolvedValue({
       id: 'upload-1',
       knowledgeBaseId: 'kb-1',
       status: 'uploading',
@@ -89,11 +87,10 @@ describe('POST /api/knowledge/[id]/documents/uploads', () => {
       workspaceId: WORKSPACE_ID,
       userId: 'user-1',
     })
-    expect(mockCreateUploadSession).toHaveBeenCalledWith({
+    expect(mockCreateKnowledgeDocumentUploadSession).toHaveBeenCalledWith({
       workspaceId: WORKSPACE_ID,
       userId: 'user-1',
       knowledgeBaseId: 'kb-1',
-      purpose: 'knowledge_document',
       fileName: 'guide.pdf',
       contentType: 'application/pdf',
       fileSize: 1024,
@@ -103,7 +100,7 @@ describe('POST /api/knowledge/[id]/documents/uploads', () => {
       },
     })
     expect(mockRequireKnowledgeDocumentUploadBilling.mock.invocationCallOrder[0]).toBeLessThan(
-      mockCreateUploadSession.mock.invocationCallOrder[0]
+      mockCreateKnowledgeDocumentUploadSession.mock.invocationCallOrder[0]
     )
   })
 
@@ -116,6 +113,6 @@ describe('POST /api/knowledge/[id]/documents/uploads', () => {
 
     expect(response.status).toBe(403)
     expect(mockRequireKnowledgeDocumentUploadBilling).not.toHaveBeenCalled()
-    expect(mockCreateUploadSession).not.toHaveBeenCalled()
+    expect(mockCreateKnowledgeDocumentUploadSession).not.toHaveBeenCalled()
   })
 })

apps/sim/app/api/knowledge/[id]/documents/uploads/route.ts

@@ -2,15 +2,17 @@ import { type NextRequest, NextResponse } from 'next/server'
 import { createKnowledgeDocumentUploadContract } from '@/lib/api/contracts/knowledge/upload-sessions'
 import { parseRequest } from '@/lib/api/server'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
-import { createUploadSession } from '@/lib/uploads/multipart-session/service'
 import { validateFileType } from '@/lib/uploads/utils/validation'
 import { uploadSessionErrorResponse } from '@/app/api/files/uploads/utils'
 import {
   requireKnowledgeDocumentUploadAccess,
   requireKnowledgeDocumentUploadActor,
   requireKnowledgeDocumentUploadBilling,
 } from '@/app/api/knowledge/[id]/documents/uploads/utils'
-import { toV2KnowledgeDocumentUpload } from '@/app/api/v2/knowledge/[id]/documents/uploads/utils'
+import {
+  createKnowledgeDocumentUploadSession,
+  toV2KnowledgeDocumentUpload,
+} from '@/app/api/v2/knowledge/[id]/documents/uploads/utils'
 
 interface KnowledgeDocumentUploadsRouteParams {
   params: Promise<{ id: string }>
@@ -40,11 +42,10 @@ export const POST = withRouteHandler(
       return NextResponse.json({ error: fileTypeError.message }, { status: 415 })
     }
     try {
-      const upload = await createUploadSession({
+      const upload = await createKnowledgeDocumentUploadSession({
         workspaceId,
         userId: actor.id,
         knowledgeBaseId,
-        purpose: 'knowledge_document',
         fileName: name,
         contentType,
         fileSize: size,

apps/sim/app/api/v2/knowledge/[id]/documents/uploads/route.test.ts

@@ -6,12 +6,12 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 const {
   mockCheckRateLimit,
-  mockCreateUploadSession,
+  mockCreateKnowledgeDocumentUploadSession,
   mockResolveKnowledgeDocumentUploadAccess,
   mockResolveKnowledgeDocumentUploadBilling,
 } = vi.hoisted(() => ({
   mockCheckRateLimit: vi.fn(),
-  mockCreateUploadSession: vi.fn(),
+  mockCreateKnowledgeDocumentUploadSession: vi.fn(),
   mockResolveKnowledgeDocumentUploadAccess: vi.fn(),
   mockResolveKnowledgeDocumentUploadBilling: vi.fn(),
 }))
@@ -20,10 +20,8 @@ vi.mock('@/app/api/v1/middleware', () => ({ checkRateLimit: mockCheckRateLimit }
 vi.mock('@/app/api/v2/lib/gate', () => ({
   v2ApiGateError: vi.fn().mockResolvedValue(null),
 }))
-vi.mock('@/lib/uploads/multipart-session/service', () => ({
-  createUploadSession: mockCreateUploadSession,
-}))
 vi.mock('@/app/api/v2/knowledge/[id]/documents/uploads/utils', () => ({
+  createKnowledgeDocumentUploadSession: mockCreateKnowledgeDocumentUploadSession,
   resolveKnowledgeDocumentUploadAccess: mockResolveKnowledgeDocumentUploadAccess,
   resolveKnowledgeDocumentUploadBilling: mockResolveKnowledgeDocumentUploadBilling,
   toV2KnowledgeDocumentUpload: (session: Record<string, unknown>) => ({
@@ -74,7 +72,7 @@ describe('POST /api/v2/knowledge/[id]/documents/uploads', () => {
       kb: { id: 'kb-1', name: 'Docs' },
     })
     mockResolveKnowledgeDocumentUploadBilling.mockResolvedValue({ actorUserId: 'user-1' })
-    mockCreateUploadSession.mockResolvedValue({
+    mockCreateKnowledgeDocumentUploadSession.mockResolvedValue({
       id: 'upload-1',
       knowledgeBaseId: 'kb-1',
       status: 'uploading',
@@ -100,11 +98,10 @@ describe('POST /api/v2/knowledge/[id]/documents/uploads', () => {
       })
     )
     expect(mockResolveKnowledgeDocumentUploadBilling).toHaveBeenCalled()
-    expect(mockCreateUploadSession).toHaveBeenCalledWith({
+    expect(mockCreateKnowledgeDocumentUploadSession).toHaveBeenCalledWith({
       workspaceId: WORKSPACE_ID,
       userId: 'user-1',
       knowledgeBaseId: 'kb-1',
-      purpose: 'knowledge_document',
       fileName: 'guide.pdf',
       contentType: 'application/pdf',
       fileSize: 1024,
@@ -114,7 +111,7 @@ describe('POST /api/v2/knowledge/[id]/documents/uploads', () => {
       },
     })
     expect(mockResolveKnowledgeDocumentUploadBilling.mock.invocationCallOrder[0]).toBeLessThan(
-      mockCreateUploadSession.mock.invocationCallOrder[0]
+      mockCreateKnowledgeDocumentUploadSession.mock.invocationCallOrder[0]
     )
   })
 
@@ -127,6 +124,6 @@ describe('POST /api/v2/knowledge/[id]/documents/uploads', () => {
 
     expect(response.status).toBe(403)
     expect(mockResolveKnowledgeDocumentUploadBilling).not.toHaveBeenCalled()
-    expect(mockCreateUploadSession).not.toHaveBeenCalled()
+    expect(mockCreateKnowledgeDocumentUploadSession).not.toHaveBeenCalled()
   })
 })

apps/sim/app/api/v2/knowledge/[id]/documents/uploads/route.ts

@@ -5,10 +5,10 @@ import { NextResponse } from 'next/server'
 import { v2CreateKnowledgeDocumentUploadContract } from '@/lib/api/contracts/v2/knowledge'
 import { parseRequest } from '@/lib/api/server'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
-import { createUploadSession } from '@/lib/uploads/multipart-session/service'
 import { validateFileType } from '@/lib/uploads/utils/validation'
 import { checkRateLimit } from '@/app/api/v1/middleware'
 import {
+  createKnowledgeDocumentUploadSession,
   resolveKnowledgeDocumentUploadAccess,
   resolveKnowledgeDocumentUploadBilling,
   toV2KnowledgeDocumentUpload,
@@ -64,11 +64,10 @@ export const POST = withRouteHandler(
         return v2Error('UNSUPPORTED_MEDIA_TYPE', fileTypeError.message)
       }
 
-      const session = await createUploadSession({
+      const session = await createKnowledgeDocumentUploadSession({
         workspaceId,
         userId,
         knowledgeBaseId,
-        purpose: 'knowledge_document',
         fileName: name,
         contentType,
         fileSize: size,

apps/sim/app/api/v2/knowledge/[id]/documents/uploads/utils.test.ts

@@ -3,18 +3,17 @@
  */
 import { NextRequest } from 'next/server'
 import { beforeEach, describe, expect, it, vi } from 'vitest'
+import type { UploadSessionRecord } from '@/lib/uploads/multipart-session/service'
 
 const {
   mockAbortUploadSession,
-  mockDeleteFile,
-  mockDeleteFileMetadata,
+  mockCreateUploadSession,
   mockFindBoundKnowledgeDocument,
   mockPerformUploadKnowledgeDocument,
   mockRecordKnowledgeBaseFileOwnership,
 } = vi.hoisted(() => ({
   mockAbortUploadSession: vi.fn(),
-  mockDeleteFile: vi.fn(),
-  mockDeleteFileMetadata: vi.fn(),
+  mockCreateUploadSession: vi.fn(),
   mockFindBoundKnowledgeDocument: vi.fn(),
   mockPerformUploadKnowledgeDocument: vi.fn(),
   mockRecordKnowledgeBaseFileOwnership: vi.fn(),
@@ -28,21 +27,21 @@ vi.mock('@/lib/knowledge/orchestration/documents', () => ({
 }))
 vi.mock('@/lib/uploads/multipart-session/service', () => ({
   abortUploadSession: mockAbortUploadSession,
+  createUploadSession: mockCreateUploadSession,
   getOwnedUploadSession: vi.fn(),
 }))
-vi.mock('@/lib/uploads/core/storage-service', () => ({ deleteFile: mockDeleteFile }))
 vi.mock('@/lib/uploads/server/metadata', () => ({
-  deleteFileMetadata: mockDeleteFileMetadata,
   recordKnowledgeBaseFileOwnership: mockRecordKnowledgeBaseFileOwnership,
 }))
 
 import {
   abortKnowledgeDocumentUpload,
+  createKnowledgeDocumentUploadSession,
   finalizeKnowledgeDocumentUpload,
 } from '@/app/api/v2/knowledge/[id]/documents/uploads/utils'
 
 const WORKSPACE_ID = '6fc7631d-88cd-46f8-9f0a-d4764daef7f8'
-const CLAIMED = {
+const CLAIMED: UploadSessionRecord = {
   id: 'upload-1',
   workspaceId: WORKSPACE_ID,
   userId: 'user-1',
@@ -66,8 +65,7 @@ const CLAIMED = {
   error: null,
   completedAt: null,
   updatedAt: new Date('2026-08-03T21:00:00.000Z'),
-  // biome-ignore lint/suspicious/noExplicitAny: partial session shape for the test
-} as any
+}
 const DOCUMENT = { id: 'upload-1', knowledgeBaseId: 'kb-1', filename: 'guide.pdf' }
 
 function finalize(resolveAttribution = vi.fn().mockResolvedValue({ actorUserId: 'payer-1' })) {
@@ -84,6 +82,60 @@ function finalize(resolveAttribution = vi.fn().mockResolvedValue({ actorUserId:
   })
 }
 
+function createSession() {
+  return createKnowledgeDocumentUploadSession({
+    workspaceId: WORKSPACE_ID,
+    userId: 'user-1',
+    knowledgeBaseId: 'kb-1',
+    fileName: 'guide.pdf',
+    contentType: 'application/pdf',
+    fileSize: 1024,
+    metadata: { tag1: 'product' },
+  })
+}
+
+describe('createKnowledgeDocumentUploadSession', () => {
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mockCreateUploadSession.mockResolvedValue(CLAIMED)
+    mockRecordKnowledgeBaseFileOwnership.mockResolvedValue(undefined)
+    mockAbortUploadSession.mockResolvedValue({ ...CLAIMED, status: 'aborted' })
+  })
+
+  it('records the ownership binding before returning the upload token', async () => {
+    await expect(createSession()).resolves.toBe(CLAIMED)
+
+    expect(mockCreateUploadSession).toHaveBeenCalledWith({
+      workspaceId: WORKSPACE_ID,
+      userId: 'user-1',
+      knowledgeBaseId: 'kb-1',
+      purpose: 'knowledge_document',
+      fileName: 'guide.pdf',
+      contentType: 'application/pdf',
+      fileSize: 1024,
+      metadata: { tag1: 'product' },
+    })
+    expect(mockRecordKnowledgeBaseFileOwnership).toHaveBeenCalledWith({
+      key: 'kb/guide.pdf',
+      userId: 'user-1',
+      workspaceId: WORKSPACE_ID,
+      originalName: 'guide.pdf',
+      contentType: 'application/pdf',
+      size: 1024,
+    })
+    expect(mockCreateUploadSession.mock.invocationCallOrder[0]).toBeLessThan(
+      mockRecordKnowledgeBaseFileOwnership.mock.invocationCallOrder[0]
+    )
+  })
+
+  it('aborts provider state when the ownership binding cannot be recorded', async () => {
+    mockRecordKnowledgeBaseFileOwnership.mockRejectedValue(new Error('database unavailable'))
+
+    await expect(createSession()).rejects.toThrow('database unavailable')
+    expect(mockAbortUploadSession).toHaveBeenCalledWith(CLAIMED)
+  })
+})
+
 describe('abortKnowledgeDocumentUpload', () => {
   beforeEach(() => {
     vi.clearAllMocks()
@@ -113,9 +165,6 @@ describe('finalizeKnowledgeDocumentUpload', () => {
   beforeEach(() => {
     vi.clearAllMocks()
     mockFindBoundKnowledgeDocument.mockResolvedValue({ status: 'absent' })
-    mockRecordKnowledgeBaseFileOwnership.mockResolvedValue(undefined)
-    mockDeleteFile.mockResolvedValue(undefined)
-    mockDeleteFileMetadata.mockResolvedValue(true)
     mockPerformUploadKnowledgeDocument.mockResolvedValue({
       success: true,
       document: DOCUMENT,
@@ -127,14 +176,6 @@ describe('finalizeKnowledgeDocumentUpload', () => {
     const result = await finalize()
 
     expect(result).toEqual({ value: DOCUMENT, completedFileId: 'upload-1' })
-    expect(mockRecordKnowledgeBaseFileOwnership).toHaveBeenCalledWith({
-      key: 'kb/guide.pdf',
-      userId: 'user-1',
-      workspaceId: WORKSPACE_ID,
-      originalName: 'guide.pdf',
-      contentType: 'application/pdf',
-      size: 1024,
-    })
     expect(mockPerformUploadKnowledgeDocument).toHaveBeenCalledWith(
       expect.objectContaining({
         documentId: 'upload-1',
@@ -144,7 +185,6 @@ describe('finalizeKnowledgeDocumentUpload', () => {
         document: expect.objectContaining({ filename: 'guide.pdf', tag1: 'product' }),
       })
     )
-    expect(mockDeleteFile).not.toHaveBeenCalled()
   })
 
   it('answers a retry from the bound document without resolving a payer', async () => {
@@ -155,32 +195,32 @@ describe('finalizeKnowledgeDocumentUpload', () => {
 
     expect(result).toEqual({ value: DOCUMENT, completedFileId: 'upload-1' })
     expect(resolveAttribution).not.toHaveBeenCalled()
-    expect(mockRecordKnowledgeBaseFileOwnership).not.toHaveBeenCalled()
     expect(mockPerformUploadKnowledgeDocument).not.toHaveBeenCalled()
-    expect(mockDeleteFile).not.toHaveBeenCalled()
   })
 
-  it('deletes the uploaded object when creation fails and nothing is bound', async () => {
+  it('retains completed bytes for retry when document creation fails', async () => {
     mockPerformUploadKnowledgeDocument.mockResolvedValue({
       success: false,
       errorCode: 'payload_too_large',
       error: 'Storage limit exceeded',
     })
 
     await expect(finalize()).rejects.toThrow('Storage limit exceeded')
-    expect(mockDeleteFile).toHaveBeenCalledWith({ key: 'kb/guide.pdf', context: 'knowledge-base' })
-    expect(mockDeleteFileMetadata).toHaveBeenCalledWith('kb/guide.pdf')
+    expect(mockFindBoundKnowledgeDocument).toHaveBeenCalledTimes(1)
   })
 
-  it('keeps the uploaded object when a document is bound despite the failure', async () => {
+  it('lets a retry converge when the first response fails after the document binds', async () => {
     mockFindBoundKnowledgeDocument
       .mockResolvedValueOnce({ status: 'absent' })
       .mockResolvedValueOnce({ status: 'bound', document: DOCUMENT })
     mockPerformUploadKnowledgeDocument.mockRejectedValue(new Error('audit sink exploded'))
 
     await expect(finalize()).rejects.toThrow('audit sink exploded')
-    expect(mockDeleteFile).not.toHaveBeenCalled()
-    expect(mockDeleteFileMetadata).not.toHaveBeenCalled()
+    await expect(finalize()).resolves.toEqual({
+      value: DOCUMENT,
+      completedFileId: 'upload-1',
+    })
+    expect(mockPerformUploadKnowledgeDocument).toHaveBeenCalledTimes(1)
   })
 
   it('rejects an upload id already bound to a different document without deleting anything', async () => {
@@ -191,6 +231,5 @@ describe('finalizeKnowledgeDocumentUpload', () => {
       'Upload id is already bound to a different document'
     )
     expect(resolveAttribution).not.toHaveBeenCalled()
-    expect(mockDeleteFile).not.toHaveBeenCalled()
   })
 })