fix(uploads): roll back folders archive extraction created

Extraction now materializes folders before uploading files, but the failure
path only deleted the extracted files — every folder the call created was left
behind. That is not cosmetic: `materialize_file` guards re-extraction by looking
up the root folder path and refusing when it has any child, so a half-extracted
nested archive turned every retry into "already extracted — delete that folder
first" until a human cleaned up the tree by hand.

The rollback must delete only folders this call actually inserted, never one it
reused: extracting into an existing path is normal (a sibling entry, an earlier
successful extraction), and deleting a pre-existing folder would destroy
unrelated user data. `ensureWorkspaceFileFolderPath` already distinguishes the
two while walking the segment chain, so it (and its application operation) now
reports `createdFolderIds` alongside the leaf id. The extractor accumulates
those ids in creation order and, on failure, deletes them in reverse — parents
are recorded before their children, so reverse order is deepest-first and a
parent is never removed out from under a child. Folder cleanup is best-effort
like the existing file cleanup, so a cleanup failure never masks the original
error.

Author: waleedlatif1

apps/sim/lib/copilot/tools/handlers/vfs-mutate.test.ts

@@ -237,7 +237,10 @@ describe('vfs mv/cp', () => {
       return mocks.getWorkspaceFileByName('ws-1', segments.at(-1), { folderId: null })
     })
     mocks.findWorkspaceFileFolderIdByPath.mockResolvedValue(null)
-    mocks.ensureWorkspaceFileFolderPath.mockResolvedValue('ensured-folder')
+    mocks.ensureWorkspaceFileFolderPath.mockResolvedValue({
+      folderId: 'ensured-folder',
+      createdFolderIds: [],
+    })
     mocks.ensureCopilotFileFolderPath.mockResolvedValue('ensured-folder')
     mocks.moveWorkspaceFileItems.mockResolvedValue({ movedItems: { files: 1, folders: 0 } })
     mocks.updateWorkspaceFileFolder.mockResolvedValue({ folder: { name: 'Reports 2025' } })

apps/sim/lib/copilot/vfs/resource-writer.test.ts

@@ -53,7 +53,10 @@ import {
 describe('resource writer', () => {
   beforeEach(() => {
     vi.clearAllMocks()
-    mocks.ensureWorkspaceFileFolderPath.mockResolvedValue('folder-id')
+    mocks.ensureWorkspaceFileFolderPath.mockResolvedValue({
+      folderId: 'folder-id',
+      createdFolderIds: [],
+    })
     mocks.admitCreateWorkspaceFile.mockResolvedValue(undefined)
   })
 
@@ -107,7 +110,10 @@ describe('resource writer', () => {
   })
 
   it('auto-creates missing parent folders for plain workspace file creates', async () => {
-    mocks.ensureWorkspaceFileFolderPath.mockResolvedValue('folder-nested')
+    mocks.ensureWorkspaceFileFolderPath.mockResolvedValue({
+      folderId: 'folder-nested',
+      createdFolderIds: [],
+    })
     mocks.createWorkspaceFileBufferByPath.execute.mockResolvedValue({
       id: 'file-report',
       name: 'summary.csv',

apps/sim/lib/uploads/archive.test.ts

@@ -16,18 +16,22 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
  * - `exactName: true` throws `FileConflictError` on a duplicate leaf name, while
  *   `exactName: false` auto-suffixes, mirroring `uploadWorkspaceFile`.
  */
-const { store, mockUpload, mockDelete, mockEnsureFolder } = vi.hoisted(() => ({
+const { store, mockUpload, mockDelete, mockEnsureFolder, mockDeleteFolder } = vi.hoisted(() => ({
   store: {
     folderIdByPath: new Map<string, string>(),
     fileKeys: new Set<string>(),
+    /** Paths passed to the folder-delete operation, in call order. */
+    deletedFolderPaths: [] as string[],
     sequence: 0,
   },
   mockUpload: vi.fn(),
   mockDelete: vi.fn(),
   mockEnsureFolder: vi.fn(),
+  mockDeleteFolder: vi.fn(),
 }))
 vi.mock('@/lib/workspace-files/application/workspace-file-folders', () => ({
   ensureWorkspaceFileFolderPathOperation: { execute: mockEnsureFolder },
+  deleteWorkspaceFileFolderOperation: { execute: mockDeleteFolder },
 }))
 vi.mock('@/lib/workspace-files/application/create-workspace-file', () => ({
   createWorkspaceFileFromBuffer: {
@@ -109,6 +113,21 @@ function allocateUniqueName(folderKey: string, name: string): string {
   }
 }
 
+/** Reverse lookup of the fake folder store: id -> path, or `undefined` if gone. */
+function folderPathById(folderId: string | undefined): string | undefined {
+  for (const [path, id] of store.folderIdByPath) {
+    if (id === folderId) return path
+  }
+  return undefined
+}
+
+/** Pre-seeds a folder chain that existed before extraction ran. */
+function seedExistingFolders(...paths: string[][]): void {
+  for (const segments of paths) {
+    store.folderIdByPath.set(buildFolderPath(segments), `preexisting_${++store.sequence}`)
+  }
+}
+
 /** Pre-seeds an already-existing workspace file so a later leaf name collides. */
 function seedExistingFile(folderId: string | null, name: string): void {
   store.fileKeys.add(`${folderId ?? ''}|${name}`)
@@ -118,11 +137,15 @@ beforeEach(() => {
   vi.clearAllMocks()
   store.folderIdByPath.clear()
   store.fileKeys.clear()
+  store.deletedFolderPaths.length = 0
   store.sequence = 0
 
   mockEnsureFolder.mockImplementation(async ({ input }: { input: { pathSegments: string[] } }) => {
     let folderId: string | null = null
     const walked: string[] = []
+    // Only the segments this call actually inserts are reported as created; a
+    // segment resolved from the store was reused and must never be rolled back.
+    const createdFolderIds: string[] = []
     for (const segment of input.pathSegments) {
       walked.push(segment)
       const path = buildFolderPath(walked)
@@ -133,10 +156,28 @@ beforeEach(() => {
       }
       folderId = `folder_${++store.sequence}`
       store.folderIdByPath.set(path, folderId)
+      createdFolderIds.push(folderId)
     }
-    return { folderId }
+    return { folderId, createdFolderIds }
   })
 
+  mockDeleteFolder.mockImplementation(
+    async ({ input }: { input: { folderId?: string; recursive?: boolean } }) => {
+      const path = folderPathById(input.folderId)
+      // Mirrors `deleteWorkspaceFileFolderOperation`, which raises `not_found` when
+      // nothing was archived — deleting a parent before its children would make the
+      // child's own delete hit this.
+      if (!path) throw new Error('Folder not found')
+      store.deletedFolderPaths.push(path)
+      for (const [candidate] of store.folderIdByPath) {
+        if (candidate === path || candidate.startsWith(`${path}/`)) {
+          store.folderIdByPath.delete(candidate)
+        }
+      }
+      return { deletedItems: { files: 0, folders: 1 } }
+    }
+  )
+
   mockDelete.mockResolvedValue(undefined)
   mockUpload.mockImplementation(
     async ({
@@ -443,6 +484,84 @@ describe('decompressArchiveBufferToWorkspaceFiles', () => {
     )
   })
 
+  it('rolls back the folders it created when an upload fails mid-extraction', async () => {
+    // `materialize_file` refuses to re-extract into a root folder that still has any
+    // child, so a folder left behind by a failed run turns every retry into
+    // "already extracted" until a human deletes the tree by hand.
+    const buffer = await buildZip({ 'a/one.txt': 'first', 'b/two.txt': 'second' })
+    mockUpload
+      .mockResolvedValueOnce({
+        file: { id: 'f_one', name: 'one.txt', url: '/one', key: 'k/one', size: 5 },
+      })
+      .mockRejectedValueOnce(new Error('storage quota exceeded'))
+
+    await expect(
+      decompressArchiveBufferToWorkspaceFiles(buffer, {
+        workspaceId: 'ws',
+        principal: TEST_PRINCIPAL,
+        rootFolderSegments: ['bundle'],
+      })
+    ).rejects.toThrow('storage quota exceeded')
+
+    expect([...store.folderIdByPath.keys()]).toEqual([])
+    expect(mockDeleteFolder).toHaveBeenCalledWith(
+      expect.objectContaining({
+        input: expect.objectContaining({ workspaceId: 'ws', recursive: true }),
+      })
+    )
+  })
+
+  it('leaves a folder that already existed before the call untouched on rollback', async () => {
+    // Extracting into an existing path is normal — a sibling entry, or an earlier
+    // successful extraction. Deleting a reused folder would destroy unrelated data.
+    seedExistingFolders(['bundle'], ['bundle', 'keep'])
+    const preexistingIds = [...store.folderIdByPath.values()]
+    const buffer = await buildZip({ 'keep/kept.txt': 'a', 'fresh/new.txt': 'b' })
+    mockUpload
+      .mockResolvedValueOnce({
+        file: { id: 'f_kept', name: 'kept.txt', url: '/kept', key: 'k/kept', size: 1 },
+      })
+      .mockRejectedValueOnce(new Error('storage quota exceeded'))
+
+    await expect(
+      decompressArchiveBufferToWorkspaceFiles(buffer, {
+        workspaceId: 'ws',
+        principal: TEST_PRINCIPAL,
+        rootFolderSegments: ['bundle'],
+      })
+    ).rejects.toThrow('storage quota exceeded')
+
+    expect([...store.folderIdByPath.keys()].sort()).toEqual(['/bundle', '/bundle/keep'])
+    expect(store.deletedFolderPaths).toEqual(['/bundle/fresh'])
+    const deletedIds = mockDeleteFolder.mock.calls.map(([args]) => args.input.folderId)
+    for (const preexistingId of preexistingIds) {
+      expect(deletedIds).not.toContain(preexistingId)
+    }
+  })
+
+  it('deletes rolled-back folders deepest-first', async () => {
+    // A parent removed before its children would make the children's own deletes
+    // fail (nothing left to archive), so the unwind walks creation order backwards.
+    const buffer = await buildZip({ 'x/y/z/leaf.txt': 'a' })
+    mockUpload.mockRejectedValueOnce(new Error('storage quota exceeded'))
+
+    await expect(
+      decompressArchiveBufferToWorkspaceFiles(buffer, {
+        workspaceId: 'ws',
+        principal: TEST_PRINCIPAL,
+        rootFolderSegments: ['bundle'],
+      })
+    ).rejects.toThrow('storage quota exceeded')
+
+    expect(store.deletedFolderPaths).toEqual([
+      '/bundle/x/y/z',
+      '/bundle/x/y',
+      '/bundle/x',
+      '/bundle',
+    ])
+    expect([...store.folderIdByPath.keys()]).toEqual([])
+  })
+
   it('does not count noise entries toward the extraction cap when they are being skipped', async () => {
     // macOS Finder zips carry a __MACOSX/._* shadow per file, doubling the raw
     // entry count. 501 files + 501 shadows = 1002 raw entries — over the

apps/sim/lib/uploads/archive.ts

@@ -7,7 +7,10 @@ import type { WorkspaceFileSecretProvenance } from '@/lib/uploads/contexts/works
 import { getFileExtension, getMimeTypeFromExtension } from '@/lib/uploads/utils/file-utils'
 import { createWorkspaceFileFromBuffer } from '@/lib/workspace-files/application/create-workspace-file'
 import { deleteWorkspaceFileOperation } from '@/lib/workspace-files/application/delete-workspace-file'
-import { ensureWorkspaceFileFolderPathOperation } from '@/lib/workspace-files/application/workspace-file-folders'
+import {
+  deleteWorkspaceFileFolderOperation,
+  ensureWorkspaceFileFolderPathOperation,
+} from '@/lib/workspace-files/application/workspace-file-folders'
 import type { UserFile } from '@/executor/types'
 
 /**
@@ -345,9 +348,14 @@ export async function decompressArchiveBufferToWorkspaceFiles(
 
   // Pass 2 — extract: the archive is proven within caps; inflate again and upload.
   // Uploads themselves can still fail mid-loop (storage/DB errors, quota crossed
-  // by another writer), so a failure rolls back every file written so far —
-  // callers and their retries must never observe a partial tree.
+  // by another writer), so a failure rolls back every file written so far *and*
+  // every folder this call materialized — callers and their retries must never
+  // observe a partial tree. Leftover folders are not cosmetic: `materialize_file`
+  // refuses to re-extract into a root folder that still has any child, so a
+  // half-extracted tree would make every retry fail until a human deletes it.
   const folderIdCache = new Map<string, string | null>()
+  /** Only folders this call inserted, in creation order — never a reused one. */
+  const createdFolderIds: string[] = []
   const extracted: UserFile[] = []
   let totalBytes = 0
   try {
@@ -365,12 +373,12 @@ export async function decompressArchiveBufferToWorkspaceFiles(
         // Ensure-semantics, not create-semantics: an archive addresses every folder
         // by its full chain, so intermediates must be materialized and any folder
         // that already exists (from a sibling entry or an earlier extraction) reused.
-        folderId = (
-          await ensureWorkspaceFileFolderPathOperation.execute({
-            principal,
-            input: { workspaceId, pathSegments: folderSegments },
-          })
-        ).folderId
+        const ensured = await ensureWorkspaceFileFolderPathOperation.execute({
+          principal,
+          input: { workspaceId, pathSegments: folderSegments },
+        })
+        folderId = ensured.folderId
+        createdFolderIds.push(...ensured.createdFolderIds)
         folderIdCache.set(folderKey, folderId)
       }
 
@@ -413,6 +421,19 @@ export async function decompressArchiveBufferToWorkspaceFiles(
         // the original error is what the caller needs to see.
       }
     }
+    // Deepest-first (creation order records parents before children), so a parent is
+    // never removed out from under a child that is still being cleaned up.
+    for (let index = createdFolderIds.length - 1; index >= 0; index--) {
+      try {
+        await deleteWorkspaceFileFolderOperation.execute({
+          principal,
+          input: { workspaceId, folderId: createdFolderIds[index], recursive: true },
+        })
+      } catch {
+        // Best-effort: a folder whose cleanup fails is still deletable by hand;
+        // the original error is what the caller needs to see.
+      }
+    }
     throw error
   }
 

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

@@ -564,18 +564,31 @@ export async function createWorkspaceFileFolder(params: {
   return mapFolderWithPath(params.workspaceId, folder)
 }
 
+/**
+ * Outcome of {@link ensureWorkspaceFileFolderPath}. `createdFolderIds` lists only the
+ * folders this call actually inserted, outermost-first, so a caller that has to unwind
+ * a partial write can delete exactly what it added (reverse the list for deepest-first)
+ * without ever touching a folder that was merely reused.
+ */
+export interface EnsureWorkspaceFileFolderPathOutcome {
+  /** Id of the deepest folder, or `null` when the path resolves to the root. */
+  folderId: string | null
+  /** Ids inserted by this call, in creation order (parents before children). */
+  createdFolderIds: string[]
+}
+
 export async function ensureWorkspaceFileFolderPath(params: {
   workspaceId: string
   userId: string
   pathSegments: string[]
-}): Promise<string | null> {
-  if (params.pathSegments.length === 0) return null
+}): Promise<EnsureWorkspaceFileFolderPathOutcome> {
+  if (params.pathSegments.length === 0) return { folderId: null, createdFolderIds: [] }
 
   // Fast path: the whole chain already exists (the common case for repeated
   // writes into known folders) — per-segment indexed lookups instead of
   // loading the workspace's entire folder table.
   const existing = await findWorkspaceFileFolderIdByPath(params.workspaceId, params.pathSegments)
-  if (existing) return existing
+  if (existing) return { folderId: existing, createdFolderIds: [] }
 
   // Load all active folders once and build a lookup keyed by "name|parentId"
   // so we can resolve existing segments without a per-segment SELECT.
@@ -597,6 +610,7 @@ export async function ensureWorkspaceFileFolderPath(params: {
   }
 
   let parentId: string | null = null
+  const createdFolderIds: string[] = []
 
   for (const rawSegment of params.pathSegments) {
     const name = normalizeWorkspaceFileItemName(rawSegment, 'Folder')
@@ -629,6 +643,7 @@ export async function ensureWorkspaceFileFolderPath(params: {
         updatedAt: created.updatedAt,
       })
       parentId = created.id
+      createdFolderIds.push(created.id)
     } catch (error) {
       if (
         error instanceof WorkspaceFileFolderConflictError ||
@@ -654,7 +669,7 @@ export async function ensureWorkspaceFileFolderPath(params: {
     }
   }
 
-  return parentId
+  return { folderId: parentId, createdFolderIds }
 }
 
 export async function updateWorkspaceFileFolder(params: {

apps/sim/lib/workspace-files/application/workspace-file-folders.ts

@@ -56,6 +56,11 @@ export interface EnsureWorkspaceFileFolderPathInput {
 export interface EnsureWorkspaceFileFolderPathResult {
   /** Id of the deepest folder, or `null` when the path resolves to the root. */
   folderId: string | null
+  /**
+   * Ids this call inserted, outermost-first — never a folder it reused. Callers that
+   * materialize a tree use it to unwind exactly their own writes on failure.
+   */
+  createdFolderIds: string[]
 }
 
 export interface UpdateWorkspaceFileFolderInput {
@@ -168,12 +173,11 @@ async function executeEnsureWorkspaceFileFolderPath(args: {
   const attribution = resolvePrincipalAttribution(args.principal, {
     workspaceBillingOwnerUserId: args.context.billedAccountUserId,
   })
-  const folderId = await ensureWorkspaceFileFolderPath({
+  return ensureWorkspaceFileFolderPath({
     workspaceId: args.context.workspaceId,
     userId: attribution.attributedUserId,
     pathSegments: args.input.pathSegments,
   })
-  return { folderId }
 }
 
 async function executeUpdateWorkspaceFileFolder(args: {

apps/sim/lib/workspace-files/application/write-workspace-file-by-path.ts

@@ -85,7 +85,7 @@ async function executeCreate({
 
   const folderUserId = await resolveFolderAttributionUserId(principal, input.workspaceId)
 
-  const folderId = await ensureWorkspaceFileFolderPath({
+  const { folderId } = await ensureWorkspaceFileFolderPath({
     workspaceId: input.workspaceId,
     userId: folderUserId,
     pathSegments: parsed.folderSegments,
@@ -145,7 +145,7 @@ async function executeCreateBuffer({
   const parsed = parseWorkspaceFileCreatePath(input.path)
   await admitCreateWorkspaceFile(principal, input.workspaceId)
   const folderUserId = await resolveFolderAttributionUserId(principal, input.workspaceId)
-  const folderId = await ensureWorkspaceFileFolderPath({
+  const { folderId } = await ensureWorkspaceFileFolderPath({
     workspaceId: input.workspaceId,
     userId: folderUserId,
     pathSegments: parsed.folderSegments,