fix(folders): close the findings from the line-by-line audit of the PR

Data integrity
- Workspace fork copied `folderId` verbatim onto the child's tables and knowledge bases. The
  column carries no workspace, so a forked row pointed at a folder owned by the SOURCE
  workspace — invisible in the fork, and mutated from under it when the source later deleted
  that folder (`ON DELETE SET NULL`). Latent until now because nothing wrote a non-null
  `folderId` for those two resources; this PR is what makes it reachable. Forked resources land
  at the root, like forked files already did

Interaction bugs
- Clearing a dead `?folderId=` wrote through the params' default `history: 'push'`, so Back
  returned to the dead URL, which healed and pushed again — the Back button was dead on that
  page. It replaces now: opening a folder is navigation, correcting a URL that never pointed
  anywhere is not
- That same heal keyed off `isLoading`, which is false for a DISABLED query, false for an
  ERRORED one, and false while `keepPreviousData` is showing the previous workspace's folders.
  In all three the list is empty or stale, so a transient fetch failure — or a workspace switch
  — threw away a perfectly good open folder. Gated on `isSuccess && !isPlaceholderData`
- "New folder" while a search was active created the folder but never showed it: the search
  filters folders too, so the new row did not match, the rename field never appeared, and the
  create read as a no-op. Both pages clear the search so the thing just created is on screen
- The drag ghost was removed only by `dragend`, which fires on the source row. The table is
  virtualized, so scrolling the source out of view mid-drag unmounted it, the event never
  arrived, and the ghost stayed on the page with every row stuck at drag opacity. Cleaned up on
  unmount as the backstop
- A drag begun in another mount highlighted every folder as a valid target — including the
  dragged folder itself — then silently did nothing on drop. It only highlights when the source
  is known and was actually checked

Correctness
- The folder-duplicate route caught 23505 across the WHOLE handler, so a unique violation
  raised while copying the workflows inside the folder was relabelled "a folder with this name
  already exists". Scoped to the folder INSERT, which is the only place the two conflicts the
  caller can act on (client-supplied `newId` replay, concurrent name take) actually arise
- The optimistic folder create derived its sort order from the workspace's WORKFLOWS regardless
  of resource type. Only the workflow tree interleaves folders and resources in one ordering
  space, so a knowledge-base or table folder was placed against an unrelated one
- A table archived between `checkAccess` and the move surfaced as a 500 with a "not found"
  body; it is a 404
- `FOLDER_RESOURCE_TYPE_BY_RESTORABLE` was a `Partial` record, so adding a folder tree without a
  mapping would compile and silently restore into the workflow tree. Total record now
- Recently Deleted paired query results to folder trees by ARRAY POSITION; reordering the const
  would have filed knowledge folders under Tables with no type error. Keyed by type
- The knowledge move's no-op guard compared against the snapshot taken when the menu opened
  rather than the live row

Performance — the module split now actually does what it claimed
- `knowledge-keys.ts` justified itself as keeping server prefetches off the ~1000-line
  `kb/knowledge` module, but `knowledge/prefetch.ts` still imported its stale-time constant from
  exactly that module, whose first line pulls the `@sim/emcn` barrel. Worse, this PR had ADDED
  the same class of edge to four prefetches via `FOLDER_LIST_STALE_TIME`/`mapFolder`
- `FOLDER_LIST_STALE_TIME`, `mapFolder`, and `KNOWLEDGE_BASE_LIST_STALE_TIME` now live beside
  their key factories, so all four server prefetches import only keys-and-constants modules.
  `mapFolder` keeps its explicit field list rather than a spread, so the cached shape cannot
  drift from a client fetch

Honesty in comments
- The KB retention `deleteFilter` narrows the restore race but does not close it — documents
  hard-deleted by `onBatch` do not come back, so a restore landing mid-batch returns an emptied
  base. Said so
- A failed folder restore leaves children active while their folders are still archived. They
  are reachable (both lists fall an unresolvable `folderId` back to the root) but they sit at
  the root until the restore is retried. Said so

Product policy made visible
- Restore deliberately does not re-enable schedules, webhooks, or chats, because archive
  overwrites their state without recording it — on production ~47% of live schedules sit at
  `disabled` and ~12% of live webhooks at `isActive: false`, so reactivating to a constant would
  be wrong far more often than right. Recently Deleted now says so at the moment of restore
  instead of leaving it to be discovered

Smaller
- Pin glyph: dropped the doubled gap, added `role='img'` so the state is announced
- "Move here" carries the folder glyph its sibling leaves do
- `useMoveTable` had stolen `useDeleteTable`'s doc block
- Dead `.returning` column and a `?? null` on a non-nullable param in `moveTableToFolder`
- Tables reads one sort source for both blocks, matching Knowledge
- New test pins that `updateFolder` refuses an archived folder — the guard that stops a delete
  from unlocking its locked subfolders

Author: waleedlatif1

apps/sim/app/api/folders/[id]/duplicate/route.ts

@@ -123,18 +123,38 @@ export const POST = withRouteHandler(
           FOLDER_RESOURCE_TYPE
         )
 
-        await tx.insert(folderTable).values({
-          id: newFolderId,
-          resourceType: FOLDER_RESOURCE_TYPE,
-          userId: session.user.id,
-          workspaceId: targetWorkspaceId,
-          name: deduplicatedName,
-          parentId: targetParentId,
-          sortOrder,
-          locked: false,
-          createdAt: now,
-          updatedAt: now,
-        })
+        try {
+          await tx.insert(folderTable).values({
+            id: newFolderId,
+            resourceType: FOLDER_RESOURCE_TYPE,
+            userId: session.user.id,
+            workspaceId: targetWorkspaceId,
+            name: deduplicatedName,
+            parentId: targetParentId,
+            sortOrder,
+            locked: false,
+            createdAt: now,
+            updatedAt: now,
+          })
+        } catch (insertError) {
+          /**
+           * Scoped to THIS insert on purpose. A 23505 here is one of two real conflicts the
+           * caller can act on: `newId` is client-supplied, so replaying a duplicate whose
+           * response was lost hits the primary key; and `deduplicateFolderName` runs before
+           * the write, so a concurrent create can still take the name in between.
+           *
+           * Catching 23505 across the whole handler instead would relabel any unique violation
+           * raised while copying the workflows inside the folder — a different constraint, on a
+           * different table — as a folder-name conflict, which is both wrong and misleading.
+           * Those keep falling through to the generic 500.
+           */
+          if (getPostgresErrorCode(insertError) !== '23505') throw insertError
+          throw new FolderDuplicationError(
+            `Folder duplication conflicted for ${sourceFolderId}`,
+            409,
+            'A folder with this name already exists in this location'
+          )
+        }
 
         const folderMapping = new Map<string, string>([[sourceFolderId, newFolderId]])
         await duplicateFolderStructure(
@@ -201,20 +221,6 @@ export const POST = withRouteHandler(
         return NextResponse.json({ error: error.message }, { status: error.status })
       }
 
-      /**
-       * `newId` is client-supplied, so replaying a duplicate whose response was lost hits the
-       * primary key rather than the name index. A name collision is equally reachable: dedup
-       * runs before the INSERT, but a concurrent create can take the name in between. Either
-       * way this is a conflict the caller can act on, not a server fault.
-       */
-      if (getPostgresErrorCode(error) === '23505') {
-        logger.warn(`[${requestId}] Folder duplication conflicted`, { sourceFolderId })
-        return NextResponse.json(
-          { error: 'A folder with this name already exists in this location' },
-          { status: 409 }
-        )
-      }
-
       if (error instanceof FolderDuplicationError) {
         logger.warn(`[${requestId}] Folder duplication rejected: ${error.message}`, {
           sourceFolderId,

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

@@ -196,13 +196,22 @@ export const PATCH = withRouteHandler(
         ) {
           return NextResponse.json({ error: 'Folder not found in this workspace' }, { status: 404 })
         }
-        await moveTableToFolder(
-          tableId,
-          table.workspaceId,
-          validated.folderId,
-          requestId,
-          authResult.userId
-        )
+        try {
+          await moveTableToFolder(
+            tableId,
+            table.workspaceId,
+            validated.folderId,
+            requestId,
+            authResult.userId
+          )
+        } catch (moveError) {
+          // The move re-asserts workspace and active state, so a miss means the table was
+          // archived between `checkAccess` and the write. That is a 404, not a server fault.
+          if (moveError instanceof Error && moveError.message.endsWith('not found')) {
+            return NextResponse.json({ error: 'Table not found' }, { status: 404 })
+          }
+          throw moveError
+        }
       }
 
       // Re-read so the response reflects both a rename and a lock change.

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

@@ -1,3 +1,4 @@
+import type { ElementType } from 'react'
 import type {
   BreadcrumbEditing,
   BreadcrumbItem,
@@ -8,7 +9,7 @@ import type { WorkflowFolder } from '@/stores/folders/types'
 export interface FolderBreadcrumbItemsOptions {
   /** Root crumb label — the page's own name ("Knowledge Base", "Tables"). */
   rootLabel: string
-  rootIcon?: React.ElementType
+  rootIcon?: ElementType
   /** Root-first ancestor chain of the open folder, from `useFolderNavigation`. */
   breadcrumbs: WorkflowFolder[]
   /** Called with the folder to open, or `null` for the workspace root. */
@@ -46,10 +47,9 @@ export function folderBreadcrumbItems({
 
   breadcrumbs.forEach((folder, index) => {
     const isCurrent = index === breadcrumbs.length - 1
+    /** The open folder is where you already are, so its crumb is not a navigation target. */
     items.push({
       label: folder.name,
-      // The current folder is where you already are, so its crumb is not a navigation
-      // target — it carries the folder's own actions instead.
       onClick: isCurrent ? undefined : () => onNavigate(folder.id),
       dropdownItems: isCurrent && currentFolderActions?.length ? currentFolderActions : undefined,
       editing: isCurrent ? currentFolderEditing : undefined,

apps/sim/app/workspace/[workspaceId]/components/folders/move-options.tsx

@@ -42,6 +42,11 @@ export interface BuildMoveOptionsParams {
  *
  * Children are indexed by parent once so the tree walk stays linear in the number of
  * folders rather than re-filtering the whole list at every level.
+ *
+ * `buildSubtree` needs no cycle guard, and that is a property of the walk rather than an
+ * oversight: it descends from the `null` root, and every folder inside a parent cycle has a
+ * parent inside that cycle, so no cycle member is ever reachable from the root. Such folders
+ * are omitted from the menu, which is the right answer — they are not valid destinations.
  */
 export function buildMoveOptions({
   folders,
@@ -131,7 +136,10 @@ export function renderMoveOption(
         {option.label}
       </DropdownMenuSubTrigger>
       <DropdownMenuSubContent>
-        <DropdownMenuItem onSelect={() => onMove(option.value)}>Move here</DropdownMenuItem>
+        <DropdownMenuItem onSelect={() => onMove(option.value)}>
+          <Folder />
+          Move here
+        </DropdownMenuItem>
         <DropdownMenuSeparator />
         {option.children.map((child) => renderMoveOption(child, onMove))}
       </DropdownMenuSubContent>

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

@@ -52,7 +52,22 @@ export function useFolderNavigation({
     folderNavUrlKeys
   )
 
-  const { data: folders = EMPTY_FOLDERS, isLoading } = useFolders(workspaceId, { resourceType })
+  const {
+    data: folders = EMPTY_FOLDERS,
+    isLoading,
+    isSuccess,
+    isPlaceholderData,
+  } = useFolders(workspaceId, { resourceType })
+
+  /**
+   * The folder list is only trustworthy enough to evict a `folderId` when the query has
+   * actually succeeded for THIS workspace. `isLoading` alone is not that signal: it is false
+   * for a disabled query (no `workspaceId`), false for an errored one, and — because
+   * `useFolders` sets `keepPreviousData` — false while showing the previous workspace's
+   * folders during a workspace switch. In all three the list is empty or stale, and healing
+   * off it would throw away a perfectly good folder.
+   */
+  const foldersAreAuthoritative = isSuccess && !isPlaceholderData
 
   const setCurrentFolderId = useCallback(
     (folderId: string | null) => {
@@ -80,9 +95,15 @@ export function useFolderNavigation({
    * 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])
+    if (!foldersAreAuthoritative || !currentFolderId || folderById.has(currentFolderId)) return
+    /**
+     * `history: 'replace'`, overriding the `push` these params default to. Opening a folder is
+     * a navigation and belongs in the back stack; correcting a URL that never pointed anywhere
+     * is not. Pushing here strands the user: Back returns to the dead `?folderId=`, which heals
+     * and pushes again, so Back never escapes the page.
+     */
+    void setFolderParams({ folderId: null }, { history: 'replace' })
+  }, [foldersAreAuthoritative, currentFolderId, folderById, setFolderParams])
 
   const breadcrumbs = useMemo(() => {
     if (!currentFolderId) return EMPTY_FOLDERS

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

@@ -1,6 +1,6 @@
 'use client'
 
-import { type DragEvent, useCallback, useMemo, useRef, useState } from 'react'
+import { type DragEvent, useCallback, useEffect, useMemo, useRef, useState } from 'react'
 import { parseFolderedRowId } from '@/app/workspace/[workspaceId]/components/folders/folder-row-id'
 import type { RowDragDropConfig } from '@/app/workspace/[workspaceId]/components/resource/resource'
 
@@ -81,6 +81,20 @@ export function useFolderRowDragDrop({
     onMoveResource,
   }
 
+  /**
+   * The ghost lives on `document.body`, but the only thing that removes it is `dragend`, which
+   * fires on the SOURCE ROW. `Resource.Table` is virtualized, so scrolling the source out of
+   * view mid-drag unmounts that row and the event never arrives — leaving the ghost stuck on
+   * the page and every row frozen at drag opacity. Clean up on unmount as the backstop.
+   */
+  useEffect(
+    () => () => {
+      dragGhostRef.current?.remove()
+      dragGhostRef.current = null
+    },
+    []
+  )
+
   const isInvalidDropTarget = useCallback((targetRowId: string, sourceRowId: string) => {
     const target = parseFolderedRowId(targetRowId)
     if (target.kind !== 'folder') return true
@@ -146,7 +160,12 @@ export function useFolderRowDragDrop({
         e.preventDefault()
         e.stopPropagation()
         e.dataTransfer.dropEffect = 'move'
-        setActiveDropTargetId(rowId)
+        /**
+         * Highlight only when the source is known and was checked. Without it every folder
+         * would light up as a valid target — including the dragged folder itself and its own
+         * descendants — and the drop would then silently do nothing.
+         */
+        if (sourceRowId) setActiveDropTargetId(rowId)
       },
       onDragLeave: (e: DragEvent<HTMLDivElement>, rowId) => {
         const relatedTarget = e.relatedTarget

apps/sim/app/workspace/[workspaceId]/components/resource/resource.tsx

@@ -64,6 +64,9 @@ export interface ResourceCell {
    * rows sort to the top of every list, and without an indicator that ordering reads as
    * arbitrary. Pinning itself is an action on the row's context menu, deliberately not a
    * hover button here.
+   *
+   * Honoured only on the plain label cell: a cell rendering custom `content` owns its own
+   * layout, and the rename field replaces the label entirely while it is open.
    */
   pinned?: boolean
 }
@@ -526,7 +529,11 @@ const CellContent = memo(function CellContent({
       {icon && <span className={cellIconNodeClass}>{icon}</span>}
       <FloatingOverflowText label={label} className={cn('block', chipContentLabelClass)} />
       {pinned && (
-        <Pin className='ml-1.5 size-[12px] shrink-0 text-[var(--text-icon)]' aria-label='Pinned' />
+        <Pin
+          className='size-[12px] shrink-0 text-[var(--text-icon)]'
+          role='img'
+          aria-label='Pinned'
+        />
       )}
     </span>
   )

apps/sim/app/workspace/[workspaceId]/home/prefetch.ts

@@ -2,8 +2,7 @@ import type { QueryClient } from '@tanstack/react-query'
 import type { FolderApi } from '@/lib/api/contracts'
 import type { ListWorkspaceFilesResponse } from '@/lib/api/contracts/workspace-files'
 import { prefetchInternalJson } from '@/app/workspace/[workspaceId]/lib/prefetch-internal-fetch'
-import { FOLDER_LIST_STALE_TIME, mapFolder } from '@/hooks/queries/folders'
-import { folderKeys } from '@/hooks/queries/utils/folder-keys'
+import { FOLDER_LIST_STALE_TIME, folderKeys, mapFolder } from '@/hooks/queries/utils/folder-keys'
 import {
   WORKSPACE_FILES_LIST_STALE_TIME,
   workspaceFilesKeys,

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

@@ -691,6 +691,13 @@ export function Knowledge() {
         name,
         parentId: parentId ?? undefined,
       })
+      /**
+       * A live search term filters the folder list too, so a brand-new "New folder" would not
+       * match it — the row never renders, the rename field never appears, and the create reads
+       * as a no-op even though it succeeded. Clear the search so the thing just created is on
+       * screen to be named.
+       */
+      setSearchQuery('')
       // Drop straight into rename: the auto-generated name is a placeholder, and the user
       // should not have to hunt for a second action to replace it.
       listRenameRef.current.startRename(folderRowId(folder.id), folder.name)
@@ -834,7 +841,10 @@ export function Knowledge() {
       const kb = activeKnowledgeBaseRef.current
       if (!kb) return
       const folderId = parseMoveOptionValue(optionValue)
-      if ((kb.folderId ?? null) !== folderId) await moveKnowledgeBaseTo(kb.id, folderId)
+      // Re-read placement from the live list: `activeKnowledgeBase` is a snapshot from when
+      // the menu opened, and a refetch since then would make the no-op check wrong.
+      const current = knowledgeBasesRef.current.find((item) => item.id === kb.id) ?? kb
+      if ((current.folderId ?? null) !== folderId) await moveKnowledgeBaseTo(kb.id, folderId)
       closeRowContextMenu()
     },
     [moveKnowledgeBaseTo, closeRowContextMenu]

apps/sim/app/workspace/[workspaceId]/knowledge/prefetch.ts

@@ -2,10 +2,8 @@ import type { QueryClient } from '@tanstack/react-query'
 import type { FolderApi } from '@/lib/api/contracts/folders'
 import type { KnowledgeBaseData } from '@/lib/api/contracts/knowledge'
 import { prefetchInternalJson } from '@/app/workspace/[workspaceId]/lib/prefetch-internal-fetch'
-import { FOLDER_LIST_STALE_TIME, mapFolder } from '@/hooks/queries/folders'
-import { KNOWLEDGE_BASE_LIST_STALE_TIME } from '@/hooks/queries/kb/knowledge'
-import { folderKeys } from '@/hooks/queries/utils/folder-keys'
-import { knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys'
+import { FOLDER_LIST_STALE_TIME, folderKeys, mapFolder } from '@/hooks/queries/utils/folder-keys'
+import { KNOWLEDGE_BASE_LIST_STALE_TIME, knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys'
 
 /**
  * Prefetches the workspace's knowledge-bases list AND its knowledge-base folder tree under