fix(folders): address the review round and the full-PR audit

Reverts the workflow restore hook — it was the wrong fix
- Greptile flagged that restoring a folder leaves schedules disabled and webhooks/chats
  inactive. The hook added last round routed workflows through `restoreWorkflow` to address
  it, but `restoreDependents` ALREADY clears exactly those columns, in bulk, inside the
  restore transaction and matched on the archive timestamp. The hook bought nothing and cost
  a per-workflow read/transaction/read OUTSIDE that transaction — roughly 1600 round trips
  for a folder of 200 workflows — plus a window where the workflows were active and the
  folder was not, and it made `restoreDependents` dead code
- What no restore path can undo is the state archive OVERWRITES: `status: 'disabled'` with
  `nextRunAt` cleared, `isActive: false`. Archive does not record what those were, so
  restoring them to a constant would re-enable a schedule the user had disabled and re-run a
  completed one. Left explicit — redeploy re-activates a schedule — matching deployment state,
  which restore also deliberately leaves off. Documented on the config rather than guessed
- Kept the genuine bug the review surfaced underneath it: `restoreWorkflow` un-archived every
  dependent by `workflowId` alone, resurrecting a webhook or chat the user had archived days
  earlier. Now matched on the workflow's own `archivedAt`, which is the semantics this feature
  documents

Bugbot: orphaned knowledge bases vanished from every view
- Knowledge filtered on a raw `folderId === currentFolderId` and never re-rooted a base whose
  folder is missing or archived, so a base restored on its own — or left behind by a partial
  cascade — was invisible everywhere. Tables already fell those back to the root

A dead `?folderId=` is now healed instead of being a dead end
- A bookmark to a deleted folder rendered the root title over an empty grid, hid everything
  actually at the root, and — worse — left every create/upload action targeting the dead id,
  filing new resources somewhere nothing could reach. `useFolderNavigation` clears it once the
  folder list resolves, which fixes Files, Knowledge, and Tables at once

Parity gaps found by auditing against Files
- Tables sorted folders newest-first on a clean URL while Files and Knowledge sorted A→Z: its
  sort params are defaulted, so the raw column was never null. Reads `activeSort` now
- On Tables you could not delete the folder you were standing in — its own row is not in the
  list, and the crumb menu offered only Rename, which also made the step-out branch
  unreachable dead code
- A failed knowledge-base move was logged and never surfaced, so a rejected move — from the
  submenu or from a drag — looked like nothing happened
- A drag begun in another mount of the page could never be dropped: `onDragOver` bailed before
  `preventDefault`, so no drop event ever fired and the `dataTransfer` fallback in `onDrop`
  was unreachable. External/foreign drags are still ignored, so an OS file dropped on the list
  cannot navigate the tab away

Files: a folder cycle crashed the page
- The folder-size roll-up recursed with no `visited` guard, so a parent/child cycle in the
  cached tree — reachable through the optimistic folder-move write — recursed until the stack
  blew and the page went blank. Also indexes children once instead of re-scanning per node

Migration 0274 is now atomic
- 0272 ends with an embedded COMMIT for its CONCURRENTLY index builds, so this file is not
  guaranteed to run inside drizzle's batch transaction. Wrapped in a DO block so the parking
  pass cannot commit without the reconcile, which would leave every mirrored folder holding a
  placeholder name
- Verified against production, where 0272 has NOT yet run, so the two apply back to back and
  this is a no-op reconcile: 1,493 legacy file folders, 0 active name duplicates, 0 orphaned
  or cross-workspace parents, and 0 id collisions with `workflow_folder` — so the insert
  cannot trip the unique index, the FKs, or the resource-type trigger

Smaller corrections
- Knowledge indexed its members for the owner comparator instead of two linear scans per
  comparison, and logs a load failure from an effect rather than the render body
- Files uses the shared root sentinel instead of a bare `'__root__'` in two places
- `restore_resource` documents that its two new folder types are unreachable until the enum in
  the copilot service's tool catalog — a different repository — widens
- Corrected a forking comment that still described file folders as a separate table

Author: waleedlatif1

apps/sim/app/api/table/[tableId]/route.test.ts

@@ -97,6 +97,7 @@ describe('PATCH /api/table/[tableId] folder moves', () => {
     expect(mockFindActiveFolder).toHaveBeenCalledWith('folder-1', 'workspace-1', 'table')
     expect(mockMoveTableToFolder).toHaveBeenCalledWith(
       'tbl_1',
+      'workspace-1',
       'folder-1',
       expect.any(String),
       'user-1'
@@ -113,7 +114,13 @@ describe('PATCH /api/table/[tableId] folder moves', () => {
 
     expect(response.status).toBe(200)
     expect(mockFindActiveFolder).not.toHaveBeenCalled()
-    expect(mockMoveTableToFolder).toHaveBeenCalledWith('tbl_1', null, expect.any(String), 'user-1')
+    expect(mockMoveTableToFolder).toHaveBeenCalledWith(
+      'tbl_1',
+      'workspace-1',
+      null,
+      expect.any(String),
+      'user-1'
+    )
   })
 
   it('leaves placement untouched when folderId is omitted', async () => {

apps/sim/app/api/table/[tableId]/route.ts

@@ -196,7 +196,13 @@ export const PATCH = withRouteHandler(
         ) {
           return NextResponse.json({ error: 'Folder not found in this workspace' }, { status: 404 })
         }
-        await moveTableToFolder(tableId, validated.folderId, requestId, authResult.userId)
+        await moveTableToFolder(
+          tableId,
+          table.workspaceId,
+          validated.folderId,
+          requestId,
+          authResult.userId
+        )
       }
 
       // Re-read so the response reflects both a rename and a lock change.

apps/sim/app/workspace/[workspaceId]/components/folders/use-folder-navigation.ts

@@ -1,6 +1,6 @@
 'use client'
 
-import { useCallback, useMemo } from 'react'
+import { useCallback, useEffect, useMemo } from 'react'
 import { useQueryStates } from 'nuqs'
 import type { ServedFolderResourceType } from '@/lib/api/contracts/folders'
 import {
@@ -67,6 +67,23 @@ export function useFolderNavigation({
     return byId
   }, [folders])
 
+  /**
+   * Heals a `?folderId=` that no longer resolves — a bookmark to a folder since deleted, or a
+   * link from someone whose workspace it was not.
+   *
+   * Without this the page is a dead end rather than a mistake: the header falls back to the
+   * root title while the list still filters on the dead id, so the user sees a page that looks
+   * like the root but is empty and hides everything actually at the root. Worse, the create
+   * and upload actions keep targeting that id, so a new resource is filed somewhere nothing
+   * can reach.
+   *
+   * Waits for `isLoading` so an empty index mid-fetch never evicts a perfectly good id.
+   */
+  useEffect(() => {
+    if (isLoading || !currentFolderId || folderById.has(currentFolderId)) return
+    void setFolderParams({ folderId: null })
+  }, [isLoading, currentFolderId, folderById, setFolderParams])
+
   const breadcrumbs = useMemo(() => {
     if (!currentFolderId) return EMPTY_FOLDERS
 

apps/sim/app/workspace/[workspaceId]/components/folders/use-folder-row-drag-drop.ts

@@ -128,7 +128,21 @@ export function useFolderRowDragDrop({
       },
       onDragOver: (e: DragEvent<HTMLDivElement>, rowId) => {
         const sourceRowId = draggedRowIdRef.current
-        if (!sourceRowId || isInvalidDropTarget(rowId, sourceRowId)) return
+        if (sourceRowId) {
+          if (isInvalidDropTarget(rowId, sourceRowId)) return
+        } else if (!e.dataTransfer.types.includes(DRAG_ROW_MIME)) {
+          /**
+           * No local source and no payload of ours — an external or foreign drag. Returning
+           * without `preventDefault` leaves the browser's default handling in place, which is
+           * what stops a dropped OS file from navigating the tab away from the app.
+           */
+          return
+        }
+        /**
+         * `dataTransfer.getData` is empty during dragover by design (the drag data store is
+         * protected until drop), so a drag that began in another mount of this page can only be
+         * recognised by its MIME type here. `onDrop` re-checks validity with the real payload.
+         */
         e.preventDefault()
         e.stopPropagation()
         e.dataTransfer.dropEffect = 'move'

apps/sim/app/workspace/[workspaceId]/files/files.tsx

@@ -62,6 +62,10 @@ import {
   timeCell,
 } from '@/app/workspace/[workspaceId]/components'
 import type { MoveOptionNode } from '@/app/workspace/[workspaceId]/components/folders'
+import {
+  parseMoveOptionValue,
+  ROOT_MOVE_OPTION_VALUE,
+} from '@/app/workspace/[workspaceId]/components/folders'
 import { FilesActionBar } from '@/app/workspace/[workspaceId]/files/components/action-bar'
 import { DeleteConfirmModal } from '@/app/workspace/[workspaceId]/files/components/delete-confirm-modal'
 import { FileRowContextMenu } from '@/app/workspace/[workspaceId]/files/components/file-row-context-menu'
@@ -378,12 +382,34 @@ export function Files() {
         directSize.set(file.folderId, (directSize.get(file.folderId) ?? 0) + file.size)
       }
     }
+    /**
+     * Children indexed once rather than re-scanning `folders` per node — the roll-up visits
+     * every folder, so the filter made this quadratic.
+     */
+    const childrenByParent = new Map<string, WorkspaceFileFolderApi[]>()
+    for (const folder of folders) {
+      if (!folder.parentId) continue
+      const siblings = childrenByParent.get(folder.parentId)
+      if (siblings) siblings.push(folder)
+      else childrenByParent.set(folder.parentId, [folder])
+    }
+
     const totalSize = new Map<string, number>()
+    /**
+     * `visiting` terminates a parent/child cycle. The optimistic folder-move write can produce
+     * one in cache, and without the guard this recurses until the stack blows and takes the
+     * whole page down — the same guard the shared folder helpers carry.
+     */
+    const visiting = new Set<string>()
     const getTotal = (folderId: string): number => {
-      if (totalSize.has(folderId)) return totalSize.get(folderId)!
-      const children = folders.filter((f) => f.parentId === folderId)
+      const cached = totalSize.get(folderId)
+      if (cached !== undefined) return cached
+      if (visiting.has(folderId)) return 0
+      visiting.add(folderId)
       const size =
-        (directSize.get(folderId) ?? 0) + children.reduce((s, c) => s + getTotal(c.id), 0)
+        (directSize.get(folderId) ?? 0) +
+        (childrenByParent.get(folderId) ?? []).reduce((sum, child) => sum + getTotal(child.id), 0)
+      visiting.delete(folderId)
       totalSize.set(folderId, size)
       return size
     }
@@ -1380,7 +1406,7 @@ export function Files() {
 
   const handleContextMenuMove = useCallback(
     async (optionValue: string) => {
-      const targetFolderId = optionValue === '__root__' ? null : optionValue
+      const targetFolderId = parseMoveOptionValue(optionValue)
       try {
         await moveItems.mutateAsync({
           workspaceId,
@@ -1767,7 +1793,7 @@ export function Files() {
         .sort((a, b) => a.sortOrder - b.sortOrder || a.name.localeCompare(b.name))
         .map((f) => ({ value: f.id, label: f.name, children: buildSubtree(f.id) }))
 
-    return [{ value: '__root__', label: 'Files', children: [] }, ...buildSubtree(null)]
+    return [{ value: ROOT_MOVE_OPTION_VALUE, label: 'Files', children: [] }, ...buildSubtree(null)]
   }, [folders, selectedFolderIds, descendantFolderIdsByFolderId])
 
   const sortConfig: SortConfig = useMemo(

apps/sim/app/workspace/[workspaceId]/knowledge/knowledge.tsx

@@ -61,7 +61,7 @@ import { useKnowledgeBasesList } from '@/hooks/kb/use-knowledge'
 import { useCreateFolder, useDeleteFolderMutation, useUpdateFolder } from '@/hooks/queries/folders'
 import { useDeleteKnowledgeBase, useUpdateKnowledgeBase } from '@/hooks/queries/kb/knowledge'
 import { usePinItem, usePinnedIds, useUnpinItem } from '@/hooks/queries/pinned-items'
-import { useWorkspaceMembersQuery } from '@/hooks/queries/workspace'
+import { useWorkspaceMembersQuery, type WorkspaceMember } from '@/hooks/queries/workspace'
 import { useDebounce } from '@/hooks/use-debounce'
 import { useDebouncedSearchSetter } from '@/hooks/use-debounced-search-setter'
 import { useInlineRename } from '@/hooks/use-inline-rename'
@@ -166,6 +166,15 @@ export function Knowledge() {
 
   const { knowledgeBases, error } = useKnowledgeBasesList(workspaceId)
   const { data: members } = useWorkspaceMembersQuery(workspaceId)
+  /**
+   * Indexed once: `ownerCell` resolves a member per row, so passing the raw array makes the
+   * owner column O(rows x members) on every rebuild. Tables already does this.
+   */
+  const membersById = useMemo(() => {
+    const byId = new Map<string, WorkspaceMember>()
+    for (const member of members ?? []) byId.set(member.userId, member)
+    return byId
+  }, [members])
   /**
    * Two pin lookups: a folder pins under `resourceType: 'folder'`, which is a different pin
    * namespace from the knowledge bases it contains, so one set cannot answer for both.
@@ -175,15 +184,23 @@ export function Knowledge() {
   const pinItem = usePinItem()
   const unpinItem = useUnpinItem()
 
-  if (error) {
-    logger.error('Failed to load knowledge bases:', error)
-  }
+  useEffect(() => {
+    if (error) logger.error('Failed to load knowledge bases:', error)
+  }, [error])
+
   const userPermissions = useUserPermissionsContext()
 
   const { mutateAsync: updateKnowledgeBaseMutation } = useUpdateKnowledgeBase(workspaceId)
   const { mutateAsync: deleteKnowledgeBaseMutation } = useDeleteKnowledgeBase(workspaceId)
 
-  const { currentFolderId, setCurrentFolderId, breadcrumbs, folders } = useFolderNavigation({
+  const {
+    currentFolderId,
+    setCurrentFolderId,
+    breadcrumbs,
+    folders,
+    folderById,
+    isLoading: foldersLoading,
+  } = useFolderNavigation({
     resourceType: FOLDER_RESOURCE_TYPE,
     workspaceId,
   })
@@ -411,9 +428,19 @@ export function Knowledge() {
   }, [folders, currentFolderId, debouncedSearchQuery, activeSort, pinnedFolderIds])
 
   const processedKBs = useMemo(() => {
-    let result = filterKnowledgeBases(knowledgeBases, debouncedSearchQuery).filter(
-      (kb) => (kb.folderId ?? null) === currentFolderId
-    )
+    /**
+     * A `folderId` that no longer names an active folder — a base restored on its own out of
+     * Recently Deleted while its folder stayed archived, or a cascade that failed partway —
+     * would otherwise match no level at all and leave the base unreachable from every view.
+     * Fall it back to the root instead. Skipped while the folder list is still loading, when
+     * an empty index would transiently drag every base to the root.
+     */
+    let result = filterKnowledgeBases(knowledgeBases, debouncedSearchQuery).filter((kb) => {
+      const folderId = kb.folderId ?? null
+      const effectiveFolderId =
+        foldersLoading || !folderId || folderById.has(folderId) ? folderId : null
+      return effectiveFolderId === currentFolderId
+    })
 
     if (connectorFilter.length > 0) {
       result = result.filter((kb) => {
@@ -469,8 +496,8 @@ export function Knowledge() {
           cmp = (a.connectorTypes?.length ?? 0) - (b.connectorTypes?.length ?? 0)
           break
         case 'owner':
-          cmp = (members?.find((m) => m.userId === a.userId)?.name ?? '').localeCompare(
-            members?.find((m) => m.userId === b.userId)?.name ?? ''
+          cmp = (membersById.get(a.userId)?.name ?? '').localeCompare(
+            membersById.get(b.userId)?.name ?? ''
           )
           break
       }
@@ -479,12 +506,14 @@ export function Knowledge() {
   }, [
     knowledgeBases,
     currentFolderId,
+    folderById,
+    foldersLoading,
     debouncedSearchQuery,
     connectorFilter,
     contentFilter,
     ownerFilter,
     activeSort,
-    members,
+    membersById,
     pinnedBaseIds,
   ])
 
@@ -497,7 +526,7 @@ export function Knowledge() {
           tokens: { label: EMPTY_CELL_PLACEHOLDER },
           connectors: { label: EMPTY_CELL_PLACEHOLDER },
           created: timeCell(folder.createdAt),
-          owner: ownerCell(folder.userId, members),
+          owner: ownerCell(folder.userId, membersById),
           updated: timeCell(folder.updatedAt),
         },
       })
@@ -521,14 +550,14 @@ export function Knowledge() {
           },
           connectors: connectorCell(kb.connectorTypes),
           created: timeCell(kb.createdAt),
-          owner: ownerCell(kb.userId, members),
+          owner: ownerCell(kb.userId, membersById),
           updated: timeCell(kb.updatedAt),
         },
       }
     })
 
     return [...folderRows, ...knowledgeBaseRows]
-  }, [visibleFolders, processedKBs, members, pinnedFolderIds, pinnedBaseIds])
+  }, [visibleFolders, processedKBs, membersById, pinnedFolderIds, pinnedBaseIds])
 
   /**
    * Rename is layered over the built rows rather than folded into the builder above, so a
@@ -783,6 +812,7 @@ export function Knowledge() {
         await updateKnowledgeBaseMutation({ knowledgeBaseId, updates: { folderId } })
       } catch (moveError) {
         logger.error('Failed to move knowledge base', moveError)
+        toast.error(getErrorMessage(moveError, 'Failed to move knowledge base'))
       }
     },
     [updateKnowledgeBaseMutation]

apps/sim/app/workspace/[workspaceId]/tables/tables.tsx

@@ -133,9 +133,14 @@ export function Tables() {
     workspaceId,
   })
 
-  if (error) {
-    logger.error('Failed to load tables:', error)
-  }
+  /**
+   * Logged from an effect, not the render body: a render-phase log fires again on every
+   * re-render while the error persists, and on each of React's double renders in dev.
+   */
+  useEffect(() => {
+    if (error) logger.error('Failed to load tables:', error)
+  }, [error])
+
   const deleteTable = useDeleteTable(workspaceId)
   const renameTable = useRenameTable(workspaceId)
   const createTable = useCreateTable(workspaceId)
@@ -268,19 +273,25 @@ export function Tables() {
       const bPinned = pinnedFolderIds.has(b.id)
       if (aPinned !== bPinned) return aPinned ? -1 : 1
 
-      // Folders carry none of the table-specific columns, so `columns`/`rows`/
-      // `owner` fall back to name rather than producing an arbitrary order.
+      /**
+       * Read from `activeSort`, not the raw params: `tablesSortParams` is defaulted, so
+       * `sortColumn` is never null and folders would sort newest-first on a clean URL while
+       * Files and Knowledge sort them A→Z. Folders also carry none of the table-specific
+       * columns, so `columns`/`rows`/`owner` fall back to name rather than an arbitrary order.
+       */
+      const col = activeSort?.column ?? 'name'
+      const dir = activeSort?.direction ?? 'asc'
       let cmp = 0
-      if (sortColumn === 'created') {
+      if (col === 'created') {
         cmp = a.createdAt.getTime() - b.createdAt.getTime()
-      } else if (sortColumn === 'updated') {
+      } else if (col === 'updated') {
         cmp = a.updatedAt.getTime() - b.updatedAt.getTime()
       } else {
         cmp = a.name.localeCompare(b.name)
       }
-      return sortDirection === 'asc' ? cmp : -cmp
+      return dir === 'asc' ? cmp : -cmp
     })
-  }, [folders, currentFolderId, debouncedSearchTerm, sortColumn, sortDirection, pinnedFolderIds])
+  }, [folders, currentFolderId, debouncedSearchTerm, activeSort, pinnedFolderIds])
 
   const processedTables = useMemo(() => {
     const query = debouncedSearchTerm.trim().toLowerCase()
@@ -452,6 +463,18 @@ export function Tables() {
         disabled: !canEdit,
         onClick: () => breadcrumbRename.startRename(folder.id, folder.name),
       },
+      {
+        label: 'Delete',
+        disabled: !canEdit,
+        /**
+         * The only way to delete the folder you are inside — its own row is not in the list.
+         * This is what makes the step-out in `handleDeleteFolder` reachable.
+         */
+        onClick: () => {
+          setActiveFolder(folder)
+          setIsDeleteFolderDialogOpen(true)
+        },
+      },
     ]
   }, [currentFolderId, folderById, canEdit, breadcrumbRename.startRename])
 

apps/sim/ee/workspace-forking/lib/copy/copy-workflows.ts

@@ -45,8 +45,9 @@ interface ResolveForkFolderMappingParams {
    * Source folder ids that will directly hold copied content (workflows); null entries
    * (root-placed content) are ignored. A source folder is copied into the target only when
    * its subtree contains at least one of these, so a fork/sync never creates folders that
-   * would end up empty. Copied workspace FILES never influence this set: they live in the
-   * separate `workspace_file_folders` entity and are flattened to root by the copy.
+   * would end up empty. Copied workspace FILES never influence this set: their folders are a
+   * separate tree (`folder` rows with `resourceType = 'file'`, which this copy only ever reads
+   * as `'workflow'`) and are flattened to root by the copy.
    */
   contentFolderIds: ReadonlyArray<string | null>
 }

apps/sim/lib/api/contracts/folders.ts

@@ -21,7 +21,7 @@ export type FolderResourceType = z.output<typeof folderResourceTypeSchema>
  * never sends the field, working against a new pod. A value that is present but not in the
  * enum is REJECTED with a 400, never silently coerced to `'workflow'`: coercing would file a
  * knowledge-base folder into the workflow tree, where the Knowledge page can never see it
- * again. This is the deploy-ordering contract — see the note in the PR description.
+ * again. This is the deploy-ordering contract, pinned by `folders.test.ts`.
  */
 export const servedFolderResourceTypeSchema = z
   .enum(['workflow', 'knowledge_base', 'table'], {