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