fix(folders): close the findings from auditing the whole release landing unit

Audited as the unit that actually ships — #6014 (pinning), #6025 (workflow folders onto the
generic table), #6037 (the engine) and this branch together against `main`, rather than this
branch against `staging`.

Authorization
- `PUT /api/folders/reorder` still operated on archived folders. `getFolderLockStatus` skips
  archived rows, so `assertFolderMutable` there was a guaranteed no-op — meaning a locked folder
  became freely reparentable the moment its parent was deleted. This branch closed exactly that
  hole on `PUT /api/folders/[id]` and left its sibling open, then widened reorder to three
  resource trees. Reordering an archived folder is also wrong on its own terms:
  `collectArchivedSubtreeIds` walks the cascade by parent, so moving a branch out of an archived
  subtree silently drops it from that folder's restore

Data loss in the purge
- `batchDeleteByWorkspaceAndTimestamp` forwarded its eligibility predicate only into the SELECT,
  never the DELETE. The `folder` target runs through it, so a restore committing between the two
  statements had its folder hard-deleted anyway — taking the placement of the children the
  restore had just brought back. The workflow purge re-checks for precisely this reason and the
  knowledge-base sweep gained the same guard earlier in this branch; `folder` had neither. The
  wrapper now re-asserts eligibility on the DELETE for every caller

Wire boundary
- `toPinnedItemApi` had no return type, so `pinned_item.resource_type` — plain `text` by design,
  against a closed contract enum — was never checked. Annotating it surfaced the real hole
  immediately: during a rolling deploy an older pod can read a pin a newer one wrote, and
  returning it would fail response validation and take the WHOLE list down rather than the one
  row. Unknown kinds are now narrowed out explicitly at the boundary instead of surviving by
  accident on a filter whose stated job is something else

Interaction
- The knowledge-base FOLDER move still compared against the snapshot taken when the menu opened.
  The resource move on that page and both Tables handlers were fixed earlier in this branch; this
  was the last one

Operational
- 0274's "re-run after drain" instruction needed a precondition. Cleanup hard-deletes from
  `folder` but never from `workspace_file_folders`, so a re-run after a purge would reinstate
  every purged file folder as a soft-deleted phantom whose files are already gone. Retention is
  far longer than any drain, so running it promptly is sufficient — but the instruction said
  nothing about ordering

Accuracy
- Four comments named symbols that do not exist or no longer apply: `useFolderCreateWithDedup`
  (never existed — it is `nextUntitledFolderName`), `collectActiveSubtreeIds`,
  `restoreFolderCascade` as the live restore path, and a claim that a server
  `createSearchParamsCache` reads the shared folder param. The shared param doc also now admits
  Files declares its own `?folderId=` rather than claiming to be the single declaration
- `FOLDER_RESOURCES.file` is unreachable at runtime — `servedFolderResourceTypeSchema` does not
  serve `'file'` — and a reader would reasonably assume otherwise. Says so, and says what routing
  file folders through the generic engine would bypass
- `pinnedItem.resourceType`'s inline comment was missing `'folder'`
- `folders.test.ts` fixtures still carried `color` and `isExpanded`, both deliberately dropped
  from the generic table — pre-consolidation rows masquerading as folders
- `getFolders` in the folder cache had one caller, in its own file

Author: waleedlatif1

apps/sim/app/api/folders/reorder/route.ts

@@ -3,7 +3,7 @@ import { folder as folderTable } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { assertFolderMutable, FolderLockedError } from '@sim/platform-authz/workflow'
 import { getPostgresErrorCode } from '@sim/utils/errors'
-import { and, eq, inArray } from 'drizzle-orm'
+import { and, eq, inArray, isNull } from 'drizzle-orm'
 import { type NextRequest, NextResponse } from 'next/server'
 import { reorderFoldersContract } from '@/lib/api/contracts'
 import { parseRequest } from '@/lib/api/server'
@@ -38,10 +38,24 @@ export const PUT = withRouteHandler(async (req: NextRequest) => {
     }
 
     const folderIds = updates.map((u) => u.id)
+    /**
+     * Archived folders are excluded here for the same reason `PUT /api/folders/[id]` excludes
+     * them: `getFolderLockStatus` skips archived rows, so `assertFolderMutable` below is a
+     * guaranteed no-op on one — meaning a locked folder becomes freely reparentable the moment
+     * its parent is deleted. Reordering an archived folder is also a correctness problem in its
+     * own right: `collectArchivedSubtreeIds` walks the cascade by parent, so moving a branch out
+     * of an archived subtree silently drops it from that folder's restore.
+     */
     const existingFolders = await db
       .select({ id: folderTable.id, workspaceId: folderTable.workspaceId })
       .from(folderTable)
-      .where(and(inArray(folderTable.id, folderIds), eq(folderTable.resourceType, resourceType)))
+      .where(
+        and(
+          inArray(folderTable.id, folderIds),
+          eq(folderTable.resourceType, resourceType),
+          isNull(folderTable.deletedAt)
+        )
+      )
 
     const validIds = new Set(
       existingFolders.filter((f) => f.workspaceId === workspaceId).map((f) => f.id)
@@ -138,7 +152,13 @@ export const PUT = withRouteHandler(async (req: NextRequest) => {
         await tx
           .update(folderTable)
           .set(updateData)
-          .where(and(eq(folderTable.id, update.id), eq(folderTable.resourceType, resourceType)))
+          .where(
+            and(
+              eq(folderTable.id, update.id),
+              eq(folderTable.resourceType, resourceType),
+              isNull(folderTable.deletedAt)
+            )
+          )
       }
     })
 

apps/sim/app/api/pinned-items/route.ts

@@ -4,7 +4,12 @@ import { getPostgresErrorCode } from '@sim/utils/errors'
 import { generateId } from '@sim/utils/id'
 import { and, eq } from 'drizzle-orm'
 import { type NextRequest, NextResponse } from 'next/server'
-import { createPinnedItemContract, listPinnedItemsContract } from '@/lib/api/contracts'
+import {
+  createPinnedItemContract,
+  listPinnedItemsContract,
+  type PinnedItemApi,
+  pinnedResourceTypeSchema,
+} from '@/lib/api/contracts'
 import { parseRequest } from '@/lib/api/server'
 import { getSession } from '@/lib/auth'
 import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -13,8 +18,22 @@ import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
 
 const logger = createLogger('PinnedItemsAPI')
 
-function toPinnedItemApi(row: typeof pinnedItem.$inferSelect) {
-  return { ...row, pinnedAt: row.pinnedAt.toISOString() }
+/**
+ * Narrows a stored row to the wire shape, dropping any row whose `resourceType` this build does
+ * not recognise.
+ *
+ * `pinned_item.resource_type` is plain `text` — deliberately, so the set of pinnable kinds can
+ * grow — while the contract is a closed enum. During a rolling deploy an older pod can therefore
+ * read a pin a newer one wrote. Returning it would fail response validation and take the WHOLE
+ * list down rather than the single row, so the unknown kind is skipped instead.
+ *
+ * `filterToActiveResources` already drops these as a side effect of not having a table to look
+ * them up in; this makes the guarantee explicit and compiler-checked at the wire boundary.
+ */
+function toPinnedItemApi(row: typeof pinnedItem.$inferSelect): PinnedItemApi | null {
+  const resourceType = pinnedResourceTypeSchema.safeParse(row.resourceType)
+  if (!resourceType.success) return null
+  return { ...row, resourceType: resourceType.data, pinnedAt: row.pinnedAt.toISOString() }
 }
 
 /** Lists the session user's pinned items in a workspace, optionally filtered to one `resourceType`. */
@@ -46,7 +65,11 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
 
   const activeRows = await filterToActiveResources(rows, workspaceId)
 
-  return NextResponse.json({ pinnedItems: activeRows.map(toPinnedItemApi) })
+  const pinnedItems = activeRows
+    .map(toPinnedItemApi)
+    .filter((item): item is PinnedItemApi => item !== null)
+
+  return NextResponse.json({ pinnedItems })
 })
 
 /** Pins a resource for the session user. Idempotent from the client's perspective: re-pinning returns 409. */
@@ -81,7 +104,18 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
       })
       .returning()
 
-    return NextResponse.json({ pinnedItem: toPinnedItemApi(created) }, { status: 201 })
+    /**
+     * Built from the validated `resourceType` rather than round-tripping the raw row through
+     * `toPinnedItemApi`: the value came from the contract enum, so narrowing here could never
+     * fail, and threading a `| null` through a path that cannot produce one would only obscure
+     * that.
+     */
+    const pinned: PinnedItemApi = {
+      ...created,
+      resourceType,
+      pinnedAt: created.pinnedAt.toISOString(),
+    }
+    return NextResponse.json({ pinnedItem: pinned }, { status: 201 })
   } catch (error) {
     if (getPostgresErrorCode(error) === '23505') {
       return NextResponse.json({ error: 'This item is already pinned' }, { status: 409 })

apps/sim/app/workspace/[workspaceId]/components/folders/search-params.ts

@@ -1,10 +1,12 @@
 import { parseAsString } from 'nuqs/server'
 
 /**
- * The open folder on any resource list built on the generic folder engine
- * (`workflow` / `file` / `knowledge_base` / `table`). Declared once here rather than per
- * feature so every foldered surface shares the same URL key and semantics, and so
- * `useFolderNavigation` and a server `createSearchParamsCache` can read the same parser.
+ * The open folder on a resource list built on the generic folder engine. Declared here rather
+ * than per feature so the surfaces that use the engine share one URL key and one set of
+ * semantics, and so a server `createSearchParamsCache` could read the same parser.
+ *
+ * Files declares its own `?folderId=` in `files/search-params.ts` — same key, same meaning, but
+ * it predates this module and has not been converged.
  *
  * Deliberately nullable (no `.withDefault`): a clean URL means the workspace root, which is
  * the only sane default and keeps a shared link short.

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

@@ -830,7 +830,11 @@ export function Knowledge() {
       const folder = activeFolderRef.current
       if (!folder) return
       const parentId = parseMoveOptionValue(optionValue)
-      if ((folder.parentId ?? null) !== parentId) await moveFolderTo(folder.id, parentId)
+      // Live placement, not the snapshot taken when the menu opened — a refetch or concurrent
+      // move in between would otherwise skip the write the user just chose. Matches the
+      // knowledge-base move below and both Tables handlers.
+      const current = foldersRef.current.find((item) => item.id === folder.id) ?? folder
+      if ((current.parentId ?? null) !== parentId) await moveFolderTo(folder.id, parentId)
       closeFolderContextMenu()
     },
     [moveFolderTo, closeFolderContextMenu]

apps/sim/hooks/queries/folders.test.ts

@@ -71,8 +71,6 @@ describe('folder optimistic top insertion ordering', () => {
         userId: 'user-1',
         workspaceId: 'ws-1',
         parentId: 'parent-1',
-        color: '#808080',
-        isExpanded: false,
         sortOrder: 5,
         createdAt: new Date(),
         updatedAt: new Date(),
@@ -83,8 +81,6 @@ describe('folder optimistic top insertion ordering', () => {
         userId: 'user-1',
         workspaceId: 'ws-1',
         parentId: 'parent-2',
-        color: '#808080',
-        isExpanded: false,
         sortOrder: -100,
         createdAt: new Date(),
         updatedAt: new Date(),

apps/sim/hooks/queries/utils/folder-cache.ts

@@ -5,7 +5,7 @@ import type { WorkflowFolder } from '@/stores/folders/types'
 
 const EMPTY_FOLDERS: WorkflowFolder[] = []
 
-export function getFolders(
+function getFolders(
   workspaceId: string,
   resourceType: FolderResourceType = 'workflow'
 ): WorkflowFolder[] {

apps/sim/lib/cleanup/batch-delete.ts

@@ -238,19 +238,28 @@ export async function batchDeleteByWorkspaceAndTimestamp({
   dbClient = db,
   ...rest
 }: BatchDeleteOptions): Promise<TableCleanupResult> {
+  /**
+   * Re-asserted on the DELETE, not just the SELECT. Every row here is soft-deleted and past
+   * retention, so a restore committing between the two statements is exactly the case that must
+   * not be hard-deleted — and for `folder` that would take the placement of the children the
+   * restore had just brought back with it. Rebuilt rather than reused from `selectChunk` because
+   * the id list already scopes the statement; only the eligibility half is re-checked.
+   */
+  const eligibility = [lt(timestampCol, retentionDate)]
+  if (requireTimestampNotNull) eligibility.push(isNotNull(timestampCol))
+  if (additionalPredicate) eligibility.push(additionalPredicate)
+
   return chunkedBatchDelete({
     tableDef,
     workspaceIds,
     tableName,
     dbClient,
+    deleteFilter: and(...eligibility),
     selectChunk: (chunkIds, limit) => {
-      const predicates = [inArray(workspaceIdCol, chunkIds), lt(timestampCol, retentionDate)]
-      if (requireTimestampNotNull) predicates.push(isNotNull(timestampCol))
-      if (additionalPredicate) predicates.push(additionalPredicate)
       return dbClient
         .select({ id: sql<string>`id` })
         .from(tableDef)
-        .where(and(...predicates))
+        .where(and(inArray(workspaceIdCol, chunkIds), ...eligibility))
         .limit(limit)
     },
     ...rest,

apps/sim/lib/folders/cascade.ts

@@ -53,7 +53,7 @@ export async function collectCascadeSubtreeIds(
  *
  * Matching on the exact `timestamp` is what stops a restore from also reviving folders
  * that were archived independently before or after — and is why this cannot reuse
- * {@link collectActiveSubtreeIds}, which by definition cannot see an archived subtree.
+ * {@link collectCascadeSubtreeIds}, which by definition cannot see an archived subtree.
  */
 export async function collectArchivedSubtreeIds(
   tx: DbOrTx,

apps/sim/lib/folders/config.ts

@@ -385,7 +385,7 @@ export const FOLDER_RESOURCES: Record<FolderResourceType, FolderResourceConfig>
       ({ archivedAt: timestamp, updatedAt: now }) satisfies Partial<typeof workflow.$inferInsert>,
     sortOrderColumn: workflow.sortOrder,
     /**
-     * Restored in bulk rather than through a `restoreChildren` hook. `restoreFolderCascade`
+     * Restored in bulk rather than through a `restoreChildren` hook. `restoreFolderChildren`
      * already matches these on the archive timestamp, so a webhook or chat the user archived
      * independently stays archived — and it does so in a fixed number of statements inside the
      * restore transaction. Routing them through `restoreWorkflow` instead would add a
@@ -438,6 +438,14 @@ export const FOLDER_RESOURCES: Record<FolderResourceType, FolderResourceConfig>
     archiveChildren: archiveWorkflowChildren,
     guardDelete: guardLastWorkflows,
   },
+  /**
+   * Present to satisfy the total `Record<FolderResourceType, …>`, but UNREACHABLE at runtime:
+   * `servedFolderResourceTypeSchema` does not serve `'file'`, and the Files surface goes through
+   * `workspace-file-folder-manager.ts` instead. Do not treat this entry as a live path — routing
+   * file folders through the generic engine would bypass the `workspace_file_folders:` advisory
+   * lock that makes its check-then-write pairs atomic, and the name rules that keep a folder name
+   * usable as a path segment.
+   */
   file: {
     resourceType: 'file',
     label: 'file',

apps/sim/lib/folders/naming.ts

@@ -16,7 +16,7 @@ type DbOrTx = Pick<typeof db, 'select'>
  * instead where they do (create, rename).
  *
  * The `" (N)"` shape deliberately matches both the client-side dedup in
- * `useFolderCreateWithDedup` and the backfill in migration 0272, so a deduped name reads the
+ * `nextUntitledFolderName` and the backfill in migration 0272, so a deduped name reads the
  * same however it was produced.
  */
 export async function deduplicateFolderName(