feat(folders): move workflow folders onto the generic folder table by waleedlatif1 · Pull Request #6025 · simstudioai/sim

waleedlatif1 · 2026-07-28T23:04:39Z

Summary

Repoints every workflow-folder read and write from workflow_folder to the generic folder table backfilled by migration 0272, scoped to resourceType = 'workflow'
Deletes createFolderRecord/updateFolderRecord/deleteFolderRecord from lib/workflows/utils.ts — dead code with no production callers, a second and less safe folder-write implementation (no lock check, no audit) shadowing orchestration/folder-lifecycle.ts
archivedAt → deletedAt; the dropped color/isExpanded columns are gone from the write paths

Notes for review

The resourceType filter is the correctness risk here, and typecheck cannot catch it. A read constrained by folder.id is safe (ids are UUIDs, they don't collide across types), but a read scoped only by workspace or parent would happily return file/kb/table folders. I audited all 39 reads: 21 were in that category and every one now carries an explicit eq(folder.resourceType, 'workflow'). Re-auditable with the grep in the commit history.
No dual-write / mirror step. An earlier plan staged this behind a legacy mirror; that was dropped as scaffolding nobody removes. This is the single cutover commit.
GET /api/folders still returns empty for non-workflow types on purpose: file folders are written by uploads/contexts/workspace, so their backfilled folder rows are a frozen snapshot and serving them would return stale data. Cutting those over is the next step.
v1 admin AdminFolder.color is kept as always-null rather than removed, so the public response shape stays stable for existing API clients.
Two tests changed intentionally: mocks repointed from schemaMock.workflowFolder to schemaMock.folder, and the test asserting the #6B7280 color default was removed along with the column.
workflow_folder is left in place, unread and unwritten. Dropping it, and re-adding the workflow.folderId FK against folder, is the contract step.

Type of Change

New feature

Testing

5,045 tests passing across workflows, folders, copilot, v1 admin, logs, uploads, and ee. Typecheck clean across apps/sim, db, testing, and platform-authz; lint and check:api-validation pass.

Checklist

Code follows project style guidelines
Self-reviewed my changes
Tests added/updated and passing
No new warnings introduced
I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

vercel · 2026-07-28T23:04:45Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
docs	Skipped		Jul 29, 2026 12:32am

cursor · 2026-07-28T23:04:50Z

PR Summary

High Risk
Broad cutover across APIs, orchestration, authz, copilot VFS, and cleanup; missing resourceType = 'workflow' on a workspace-scoped query could mix non-workflow folder rows, and the new sibling unique constraint changes create/import/restore behavior.

Overview
Workflow folder storage moves off workflow_folder onto the shared folder table (post-migration 0272). Every query and write is scoped with resourceType = 'workflow'; soft delete uses deletedAt instead of archivedAt. Inserts no longer set dropped columns (color, isExpanded).

Naming and conflicts: Shared deduplicateFolderName backs duplicate-folder and restore flows. Create/update/restore map Postgres 23505 to 409; folder updates also return 409 for conflict via folderMutationStatus. Admin workspace import uses ensureImportFolder (mkdir-p reuse) so path imports don’t fail on the new active sibling unique index.

Cleanup and API surface: Soft-delete hard-cleanup only deletes folder rows with resourceType = 'workflow'. Dead folder CRUD helpers are removed from lib/workflows/utils.ts. toFolderApi / listFoldersForWorkspace query folder directly. v1 admin AdminFolder.color is always null for backward compatibility. List/create contracts still only expose workflow folders until other resource writers land.

^{Reviewed by Cursor Bugbot for commit 64cb20d. Configure here.}

greptile-apps · 2026-07-28T23:09:55Z

Greptile Summary

Migrates workflow folders from workflow_folder onto the shared folder table with resourceType = 'workflow'.

Repoints API, orchestration, authz, cleanup, copilot, admin, and fork paths to folder / deletedAt
Scopes cleanup hard-delete with additionalPredicate so only workflow folders are purged
Drops dead color/isExpanded write fields on update; keeps admin color as always-null for wire stability

Confidence Score: 5/5

Prior Greptile findings are fixed on HEAD; the PR appears safe to merge.

Cleanup scopes hard-delete to workflow folders; authz and folder lifecycle id paths require resourceType=workflow; update no longer writes removed color/isExpanded columns.

Important Files Changed

Filename	Overview
apps/sim/background/cleanup-soft-deletes.ts	Folder cleanup target now ANDs resourceType=workflow via additionalPredicate; prior cross-type purge fixed.
packages/platform-authz/src/workflow.ts	isFolderInWorkspace and lock walks require resourceType=workflow on generic folder rows.
apps/sim/lib/workflows/orchestration/folder-lifecycle.ts	Cutover to folder table; color/isExpanded removed from update payload; create/update/delete/restore scoped by resourceType.
apps/sim/lib/cleanup/batch-delete.ts	Adds optional additionalPredicate into soft-delete batch selection for shared tables.
apps/sim/lib/folders/queries.ts	listFolders and toFolderApi read generic folder with resourceType filter and deletedAt mapping.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
API[Folder / workflow APIs] --> Orch[folder-lifecycle / workflow-lifecycle]
Orch --> Folder[(folder table resourceType=workflow)]
Authz[platform-authz isFolderInWorkspace] --> Folder
Cleanup[cleanup-soft-deletes] -->|additionalPredicate workflow only| Folder
Legacy[(workflow_folder unread)] -.->|left in place| Folder

_{Reviews (6): Last reviewed commit: "fix(folders): finish the unique-name han..." | Re-trigger Greptile}

waleedlatif1 · 2026-07-28T23:23:09Z

@greptile

waleedlatif1 · 2026-07-28T23:23:09Z

@cursor review

waleedlatif1 · 2026-07-28T23:32:58Z

@greptile

waleedlatif1 · 2026-07-28T23:32:59Z

@cursor review

waleedlatif1 · 2026-07-28T23:44:46Z

@greptile

waleedlatif1 · 2026-07-28T23:44:47Z

@cursor review

waleedlatif1 · 2026-07-28T23:53:54Z

@greptile

waleedlatif1 · 2026-07-28T23:53:55Z

@cursor review

waleedlatif1 · 2026-07-29T00:05:36Z

@greptile

waleedlatif1 · 2026-07-29T00:05:37Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 64cb20d. Configure here.}

createFolderRecord, updateFolderRecord and deleteFolderRecord in lib/workflows/utils.ts had no production callers — the only references were mock entries in @sim/testing that no test consumed. They were a second, less safe implementation of folder writes (no lock check, no audit) shadowing the real one in orchestration/folder-lifecycle.ts. Removing them leaves folder-lifecycle.ts as the single orchestration chokepoint and cuts the live workflow_folder write sites from 15 to 11.

Repoints every workflow-folder read and write from `workflow_folder` to the generic `folder` table backfilled by migration 0272, scoped to `resourceType = 'workflow'`. - 11 write sites and 39 read sites repointed; the 21 reads that select by workspace or parent rather than by id now carry an explicit `resourceType` filter so they cannot return another type's folders - `archivedAt` becomes `deletedAt`; the dropped `color`/`isExpanded` columns are gone from the write paths - v1 admin `AdminFolder.color` is retained as always-null so the public response shape stays stable rather than silently losing a field - `GET /api/folders` still returns empty for non-workflow types: file folders are written by `uploads/contexts/workspace` and their backfilled rows are a frozen snapshot until that writer moves too

Review + cleanup pass over the cutover. - id-keyed lookups were left unfiltered on the theory that UUIDs cannot collide across resource types. That rules out accidental collision but not a caller passing a file/kb/table folder id as a workflow folderId — isFolderInWorkspace and the folder PUT/DELETE/parent checks would have accepted one. Every folder query now carries the resourceType filter - the soft-delete cleanup job pointed at the shared folder table with no resource scope, so it would have hard-deleted file/kb/table rows the workflow cleanup does not own; batchDeleteByWorkspaceAndTimestamp gains an additionalPredicate and the folder target uses it - drop lib/folders/cascade.ts and config.ts: unreferenced, and a second recursive cascade engine over the same table that duplicates folder-lifecycle's. They belong with their first consumer - narrow listFoldersQuerySchema.resourceType to the servable set instead of accepting the full enum and silently answering "you have none", matching the create and reorder bodies - de-alias `folder as workflowFolder` to `folderTable`; collapse six doubled resourceType predicates; drop the orphaned CreateFolderInput and restore TSDoc lost to the dead-code delete

performUpdateFolder still copied color and isExpanded into its update payload after the cutover, so a caller passing either wrote against columns the generic folder table does not have. The create path had already dropped them. Typed the payload as Partial<typeof folder.$inferInsert> instead of Record<string, unknown> — the loose type is the reason typecheck caught every other dropped-column reference in this cutover but not this one.

The generic folder table carries a partial unique index on active (workspaceId, resourceType, parentId, name) that workflow_folder never had, so duplicate sibling names are newly rejectable — and prod has 35 such groups today, deduped only by the migration backfill. Create, update and restore all reached the constraint with no 23505 handling: a duplicate name surfaced as an unhandled 500. Restore had no try/catch at all, and it is the easiest one to hit, since clearing deletedAt brings a row back under the index against siblings that may have taken the name meanwhile. All three now map the violation to a conflict the caller can act on.

A 409 is the right answer when the user is choosing a name — create and rename keep it. Restore is different: the caller cannot rename an archived folder, so refusing on a name a sibling has since taken leaves them permanently unable to restore it, with no affordance to resolve the conflict. Restore now deduplicates to "<name> (N)" instead, matching the convention already used by the duplicate route, the client-side create dedup, and the 0272 backfill. The rename happens while the row is still archived, which cannot collide because the unique index only covers active rows. Only the restore root can conflict — descendants come back alongside the siblings they were archived with. Extracts deduplicateFolderName out of the duplicate route into lib/folders/naming.ts now that it has a second caller.

Three gaps left by the previous pass, all from the same new constraint. - restore re-roots a folder whose parent is still archived, but the dedup ran against the original parentId, so a root-level clash was missed and clearing deletedAt still hit the index — defeating the dedup in exactly the case it exists for. It now dedups against the resolved parent - the folder PUT handler mapped only not_found and validation, so the conflict errorCode added for renames fell through to 500. It now shares folderMutationStatus with the POST route - admin workspace import inserted path segments unconditionally, so a segment matching an existing folder failed the import. It now reuses the existing folder (mkdir -p), which is also the semantics importing into a folder path should have — the same path twice lands in one tree

…ing 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

…bles folders (#6045) * feat(folders): cut file folders over, and give knowledge bases and tables folders Builds on the generic resourceType-driven folder engine to finish the migration and extend it to two more resource trees. File-folder cutover - Repoints all 30 `workspace_file_folders` query sites onto `folder` scoped to `resourceType = 'file'`, id-keyed lookups included — a caller can hand a workflow folder's id to a file-folder endpoint, and only the predicate stops it - Adds the missing restore-conflict handling: a file folder whose name was taken while it was archived now deduplicates against the RESOLVED parent (restore re-roots when the original parent is still archived) instead of being permanently unrestorable - The forking mapper's `resourceType` predicate lives in the `leftJoin` condition, not the WHERE, where it would silently make the outer join inner - `servedFolderResourceTypeSchema` gains `file`; the soft-delete sweep enumerates all four types rather than dropping its predicate Knowledge Base and Tables folders - `knowledge_base.folder_id` and `user_table_definitions.folder_id` are now written and served; both create/move paths admit a folder only through `findActiveFolder`, scoped to workspace AND resourceType - Restoring a knowledge base whose folder is still archived re-roots it rather than filing it somewhere the page never renders; a workspace move re-roots for the same reason - Both pages get folder rows, breadcrumbs, inline rename on rows and on the breadcrumb, move-to submenus, pin/unpin, and drag-a-row-onto-a-folder Shared folder UI - One `components/folders/` module now backs Knowledge, Tables, and Files: `folderBreadcrumbItems` (data for `Resource.Header`, which owns the crumb chrome), `folderRow`, `folderRowId`/`parseFolderedRowId`, `buildMoveOptions` / `buildDescendantIndex` / `renderMoveOptions`, `nextUntitledFolderName`, `FolderContextMenu`, `useFolderNavigation`, `useFolderRowDragDrop` - Deletes the per-page copies: `files/move-options.tsx` and the Tables-local folder context menu Recently Deleted - Restores folders of every type, not just workflow folders, driven by one declarative table rather than a branch per resource Folder pins resolve against `folder` unscoped: file, knowledge-base, and table folders are all pinnable and all live there, and a folder id addresses exactly one row. * fix(folders): close the folder-engine audit findings Data loss - Migration 0274 catches up file folders created AFTER 0272. That backfill is guarded by `WHERE NOT EXISTS (… resource_type = 'file')`, so it fires once; every file folder created between it and this cutover exists only in `workspace_file_folders`. Without the catch-up they vanish from Files on deploy and their contents become unreachable Authorization - Archived folders were mutable, which bypassed the folder lock: the folder routes and `updateFolder` filtered id + resourceType but never `deletedAt`, while `getFolderLockStatus` does — so an archived-but-locked folder reported unlocked. Lock a subfolder, delete its parent, and any write-level member could rename and reparent it. PUT and the engine now require an active row; DELETE deliberately still does not, because it reuses an archived folder's own `deletedAt` to make a partial cascade retryable - `v1/admin/workflows/import` wrote `folderId` with no validation at all. Migration 0272 dropped the FK, so it accepted a knowledge-base folder, another workspace's folder, or garbage — and the workflow then rendered under no sidebar node while still executing, billing, and escaping the folder delete cascade. It now runs the same `assertFolderInWorkspace` + `assertFolderMutable` pair as `v1/workflows/import` Restore no longer loses state - A folder restore ran a bare `archivedAt = null` over its workflows, while the delete had routed through `archiveWorkflow` — which also disables schedules, webhooks, chats, and MCP tools. Restoring a folder reported success while every schedule stayed disabled forever and every chat 404'd. The workflow tree now restores through `restoreWorkflow`, like the knowledge-base and table trees already did. Deployment state is still deliberately not revived, matching the single-workflow restore - Those canonical restores re-root a resource whose folder is archived — correct on its own, catastrophic inside a cascade, which restores children BEFORE the folder rows and would therefore dump every child at the workspace root. `resolveRestoredFolderId` takes the subtree being restored and exempts it. `restoreTable` gained the re-root check it lacked Correctness - `getFolderPath` in `lib/folders/tree.ts` walked `parentId` with no `visited` set, so a cycle in the client cache — reachable through `useReorderFolders`' unchecked optimistic write — hung the tab. Its twin already had one. Dead `buildFolderMap` deleted - The sidebar's "New folder" inside a folder hardcoded the name, so the second one 409'd, was swallowed with only a log line, and the user got no folder and no message. It now names through the existing `generateSubfolderName` and surfaces failures - `chunkedBatchDelete` deleted by id with no re-check, so a resource restored between the SELECT and the DELETE was hard-deleted anyway. Callers now re-assert their soft-delete predicate on the DELETE - Recently Deleted and the `restore_resource` tool cover every folder tree, not just workflow folders, so deleted knowledge-base and table folders are recoverable Cleanup - The folder duplicate route stops steering control flow through `throw new Error('literal')` matched by string, reuses the engine's `nextFolderSortOrder` instead of recomputing it, and names its workflow scope once - `servedFolderResourceTypeSchema` documents that its default applies only to an omitted value; a present-but-unrecognized one is rejected, never coerced to `workflow` - The `workspace_file_folders` comment described the table as unread and unwritten and invited a DROP. It was live until this cutover, and it is now the rollback copy — the comment says so, and names what must be true before anyone drops it - Pinned rows carry a small non-interactive pin glyph again; they sort to the top and had nothing explaining why since the inline pin button was removed Tests - New `lib/folders/lifecycle.test.ts` covers create/update/delete/restore, the re-root, and the 23505 mappings; `cleanup-soft-deletes.test.ts` now pins the folder target's resourceType predicate, which is what keeps the sweep off a not-yet-cut-over type * improvement(folders): show the pin glyph on Files rows and pin the deploy contract Files rows were left without the pin indicator the other two lists gained, so a pinned file or folder still sorted to the top of the Files page with nothing explaining why. Adds `lib/api/contracts/folders.test.ts`, which pins the three cases that together ARE the rolling-deploy contract: an omitted `resourceType` defaults to `workflow` (so a client predating the field keeps working), a present-but-unrecognized one is rejected rather than coerced, and every served tree is accepted. * fix(folders): close the findings from the full-diff audit Migration 0274 reconciles instead of catching up - The deployed manager writes `workspace_file_folders` EXCLUSIVELY, so `folder`'s file rows are frozen at the 0272 snapshot: every rename, move, delete, and restore since then lives only in the legacy table. An insert-only catch-up left all of that diverged — renames reverted, deleted folders came back as active phantoms, and a stale-active row could hold the name a newer folder legitimately took - That last case was silent: the bare `ON CONFLICT DO NOTHING` swallows a NAME violation as readily as an id one, so the newer folder was discarded and every file inside it stranded. The conflict target is now `(id)`, so a name collision — which would mean the source violated its own unique index — fails loudly instead - Reconciles by parking mirrored names first, then upserting, so no transient state can collide with the partial unique index. Both statements are idempotent, and the header says what nothing else would: this has to be run again after the deploy drains, because old pods keep writing the legacy table until they are gone `/api/folders` no longer serves file folders - Adding `'file'` to `servedFolderResourceTypeSchema` opened a second writer over the same rows. It bypassed the `workspace_file_folders:${workspaceId}` advisory lock that makes the manager's check-then-write pairs atomic, and it accepted names containing `/`, `\`, `.` and `..`, which the file surface forbids because a folder name becomes a path segment — a folder named `a/b` is then unreachable by path forever, and breadcrumbs resolve it as two segments. The storage cutover never needed a second API, so there isn't one Conflicts that returned 500 - `v1/admin/workspaces/[id]/import` created its folder with an unserialized SELECT-then- INSERT, so two imports sharing a folder path failed the whole import. The name is server-chosen, so it now adopts whichever folder won, like `ensureWorkspaceFileFolderPath` - Folder duplicate never caught 23505. `newId` is client-supplied, so replaying a duplicate whose response was lost hit the primary key and returned 500 instead of a 409 the caller can act on Performance - `knowledgeKeys` moved to `hooks/queries/utils/knowledge-keys.ts`. Reading it from `hooks/queries/kb/knowledge` pulled a ~1000-line hook module — and the `@sim/emcn` barrel behind it — into `hooks/queries/folders`, which three server prefetches import. That is the same import edge the navigation-speed audit measured at 11.8MB per workspace route; `tableKeys` already lived in a keys-only module for exactly this reason Merge drop - Knowledge never got the chrome Tables did: its prefetch now fetches the folder tree alongside the bases, so the list does not paint ungrouped and a `?folderId=` deep link does not render an empty breadcrumb, and its loading fallback declares the "New folder" action so the header does not shift on hydration Comments corrected to match reality - The shared folder row and context menu claimed Files as a consumer; Files still builds its own row and routes folders through `FileRowContextMenu`, which the TSDoc now says - `schema.ts` prescribed a row COUNT comparison before dropping the legacy table. The divergence 0274 repairs is in row CONTENTS, which a count check passes straight through * 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 - Validated with a read-only dry run: no active name duplicates, no orphaned or cross-workspace parents, and no 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 * docs(folders): record the 0272 + 0274 dry-run results on the migration A read-only dry run materialises the complete post-0272 `folder` table from both source tables and asserts every constraint it declares: no NULL names, no primary-key collisions between the two source tables, no unique-index violations, no resource-type or workspace trigger violations, and no parent/user/workspace FK violations. 0272's dedup renames only workflow folders, never file folders — the latter is what lets pass 2 write raw names. * 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. A deliberately-disabled schedule or webhook is a common state rather than an edge case, so reactivating to a constant would be wrong 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 * fix(tables): re-read live placement before skipping a move as a no-op `handleMoveTable` and `handleMoveFolder` decided "already there, nothing to do" from `activeTable`/`activeFolder`, which are snapshots taken when the context menu opened. A refetch or a concurrent move since then makes that comparison stale, so the write the user just asked for is silently skipped and the row stays where it was. Both now compare against the live list, matching the knowledge-base move. * chore(folders): drop production figures from migration and code comments This repository is public. The 0274 header and the Recently Deleted policy comment carried concrete counts and ratios taken from the production database, which is operational detail that does not belong in open source. Both now state the property that matters — what was asserted, and why disabled-on-restore is the safe default — without the underlying numbers. * 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 * fix(folders): gate the orphan fallback on a resolved folder index, not a loading flag The URL heal was fixed to use `isSuccess && !isPlaceholderData`, but the list filters that decide "this resource's folderId does not resolve, so show it at the root" were left on `isLoading` — the same footgun, one layer down. `isLoading` is false for a disabled query (no workspaceId), false for an errored one, and — because `useFolders` sets `keepPreviousData` — false while the previous workspace's folders are still on screen during a switch. In all three the index is empty or belongs to another workspace, so every foldered knowledge base and table was treated as an orphan and dragged to the root, or filtered out of a deep-linked folder entirely. A transient fetch failure was enough to make a folder look empty. `useFolderNavigation` now exposes `foldersResolved` instead of `isLoading`, so the heal and both list filters share one signal and the footgun is no longer reachable from the hook's surface.