feat(folders): add the generic resourceType-driven folder engine

[waleedlatif1]

Summary

Extracts the generic, resourceType-driven folder engine that knowledge bases and tables need to have folders. This is the shared layer only — no KB or Tables UI. Two follow-up PRs build on it in parallel.

Before this, folder create/update/delete/restore/reorder existed once in code, hardcoded to resourceType: 'workflow'. Rather than adding a switch with four backends, the per-type deltas are captured as data:

• apps/sim/lib/folders/config.ts — FOLDER_RESOURCES declares, per resource type: the child table, its id / folderId / workspace / soft-delete columns, the TS property key backing that soft-delete column (drizzle's .set() takes property names while .where() takes column objects, and they genuinely differ — archivedAt vs deletedAt), the membership scope, an audit label, and a cascade count key. Every .set() payload is built by a per-resource closure written with satisfies Partial<typeof table.$inferInsert>, so a dropped or renamed column is a compile error at the config rather than a silent no-op.
• apps/sim/lib/folders/cascade.ts — archive/restore under one shared timestamp. Restore matches that timestamp exactly, which is what stops it from reviving resources that were deleted independently before or after. Statement count is fixed regardless of subtree depth: one subtree SELECT, one folder UPDATE, one child UPDATE, one per declared dependent table.
• apps/sim/lib/folders/lifecycle.ts — the single createFolder / updateFolder / deleteFolder / restoreFolder implementation. lib/workflows/orchestration/folder-lifecycle.ts drops from 620 lines to thin workflow-bound wrappers, so every existing caller (folders API, copilot workflow tools, resource-restore orchestrator) is untouched.

Workflow-specific behavior that genuinely cannot be a row update — archiving through archiveWorkflowsByIdsInWorkspace so deployments deactivate, external webhooks tear down, the socket is notified and MCP tool lists republish; the last-workflow-in-workspace delete guard; restoring schedules / webhooks / chats / MCP tools — is declared as data (archiveChildren, guardDelete, restoreDependents) on the workflow entry. Knowledge bases and tables use the default path entirely.

The contract widens to workflow | knowledge_base | table, and resourceType is threaded through all four routes and the React Query hooks. Query keys now include it (folders → list → <resourceType> → <workspaceId> → <scope>), so the three folder trees stop sharing one cache entry.

Behavior change: folder restore no longer reactivates independently-archived dependents

This is the highest-risk item in the PR and it touches an already-shipped path. Read this one carefully.

Folder restore previously reactivated all schedules, webhooks, chats, and MCP tools belonging to a restored workflow, regardless of each row's own archivedAt. The generic cascade requires an exact timestamp match, consistent with how it treats folders and resources.

For the ordinary case nothing changes: archiveWorkflow stamps dependents with options.archivedAt, so everything archived by a folder delete shares the folder's timestamp and comes back together. The difference is a webhook (or schedule, or chat) a user disabled before the folder was deleted — the old code silently re-enabled it on restore; it now stays archived.

I believe the new behavior is correct: it preserves a deliberate user action the old code undid, and silently re-enabling a webhook someone turned off is the more dangerous of the two failure modes. But it is a semantic change nobody asked for, so it should be an explicit decision rather than something a reviewer discovers. If you'd rather not take it in this PR, the fallback is small and local: restore dependents unfiltered for the workflow config only, leaving kb/table on the timestamp-matched path.

assertFolderMutable is gated to workflow — deliberately

Folder locking is a workflow-only feature. folder.locked exists solely because workflow-folder locking shipped before the generic table, and the schema comment already states it is not extended to the other resource types. The routes now call assertFolderMutable only when resourceType === 'workflow'.

This is intent, not an oversight: kb and table folders have no lock semantics. Calling the lock check for them would be harmless today (their locked is always false) but would imply a feature that does not exist, and would quietly start enforcing if anything ever wrote the column. If kb/table folders should be lockable, that is a separate feature with its own UI.

Unique-index handling on every newly-reachable path

The folder table has a partial unique index on active (workspaceId, resourceType, coalesce(parentId,''), name). Every path this PR makes reachable for kb/table follows the established rule — 409 where the user picks the name, deduplicateFolderName where they cannot:

Path	Handling
Create	409 conflict (user chose the name)
Rename (PUT)	409 conflict
Reparent (PUT)	409 conflict
Reparent via reorder	409 conflict — newly added; this path previously fell through to a 500
Restore	deduplicateFolderName against the resolved parent (a folder whose parent is still archived is re-rooted first), with a 23505 → 409 fallback for the create-races-restore window

The reorder case is a real fix, not just plumbing: PUT /api/folders/reorder accepts parentId, so a drag that moves a folder into a sibling set that already has that name hit the constraint and returned 500 Failed to reorder folders. It now returns a 409 with an actionable message, for workflow folders too.

Latent bug fix: the folder cycle check could escape its own tree

checkForCircularReference in lib/workflows/utils.ts walked the parent chain with eq(folder.resourceType, 'workflow') hardcoded. Now that folder ids of other resource types exist, that walk could be handed a knowledge-base folder id, find no row, terminate early, and report "no cycle" — or with the filter removed, walk out of one resource's tree into another's. It moves to lib/folders/queries.ts as wouldCreateFolderCycle(folderId, parentId, resourceType), scoped to the caller's type. The old workflow-only copy is deleted, so there is no second implementation to drift.

Review round 1 — three divergences from the canonical delete paths, all fixed

All three findings said the same thing in different places: the generic row-update path was not a faithful stand-in for what each resource does when deleted on its own. Fixed in 62975bf4c by delegating to the canonical functions rather than replicating their writes.

1. Knowledge bases lost their dependent graph (Bugbot, High). deleteKnowledgeBase also archives the KB's document rows and sets its knowledge_connector rows to archivedAt + status: 'paused'. The bare knowledge_base row update left that graph live — connectors would have kept syncing into a KB the UI reported as deleted. Restore was worse: clearing the tombstone directly would trip kb_workspace_name_active_unique and abort the whole restore transaction on any name collision, where the canonical restore renames.

2. Folder delete bypassed table delete locks (Bugbot, Medium). deleteTable gates archiving on deleteLocked inline in its WHERE, because archiving destroys access to every row. Deleting the folder around a locked table was a way around that control. The table config now declares a guardDelete that refuses the whole folder up front — checked across the entire subtree, so the cascade never archives half the tables and then stops at a locked one — and surfaces 423, matching TableLockedError.statusCode rather than inventing a different status for the same condition.

3. Re-deleting an archived folder stranded its children permanently (Greptile, P1). A second DELETE on an already-archived folder stamped still-active children with a new timestamp while the folder row kept its original (the folder UPDATE is guarded by isNull(deletedAt)). Restore matches the folder's stamp, so those children could never come back. deleteFolder now reuses the folder's existing deletedAt, which also heals the stragglers into the original snapshot instead of orphaning them — and makes a repeated delete idempotent.

Supporting changes:

• deleteKnowledgeBase and deleteTable gained an optional options.archivedAt, so a bulk caller can stamp one shared timestamp. This mirrors archiveWorkflow's existing option of the same name — the precedent the workflow cascade already depends on. Defaulted to now; every existing single-resource caller is unchanged.
• Added the restoreChildren hook that pairs with archiveChildren. Their asymmetry was the underlying defect: an archive hook exists precisely because archiving touches more than the child row, so it requires a matching restore. cascade.test.ts now asserts that pairing across FOLDER_RESOURCES, so a future resource declaring one without the other fails the suite.
• Hook-based restores run after the folder transaction commits. The canonical restores open their own transactions (with FOR UPDATE and collision-rename retries); nesting them would hold folder-row locks across a second connection. The default row-update path still runs inside the transaction.
• One shared folderMutationStatus. The routes had three copies of the code→status mapper and the DELETE handler's was an inline ternary covering only two codes — locked would have returned 500 there even with the guard working. It lives in lib/folders/status.ts, deliberately outside lifecycle.ts so route tests that mock the lifecycle module don't stub out a pure mapping function.
• resourceType is now a required argument on deduplicateFolderName and listFoldersForWorkspace. Both defaulted to 'workflow', which is exactly the silent-wrong-answer footgun that would bite the KB/Tables follow-ups: forget the argument and you dedup against the wrong tree, then fail on the unique index. Now a compile error.

Review round 2 — cascade ordering made partial failures permanent

Both round-2 findings were the same defect in opposite directions, and both were introduced by round 1: the hooks that walk resources one at a time through their canonical delete/restore can fail partway, and the cascade was ordered so that a mid-loop failure could never be retried into a correct state. Fixed in 7fe4e0373.

Delete stamped children before folders. A failure left some children soft-deleted at T1 with the folder still active, so the retry minted a fresh T2 and the T1 children fell outside every future restore. This is also why the round-1 P1 fix was incomplete on its own — reusing the folder's existing stamp only helps if the folder already has one. Folders are now stamped first, so the retry reuses T1 and sweeps the stragglers into the original snapshot.

Restore had the mirror image. Folders came back first, so a later child failure left the root's deletedAt cleared, and every retry short-circuited on the already-restored early return — reporting success with zero counts while the remaining resources stayed archived. Silent partial success is the worst possible shape for this. Hook-driven child restores now run before the folder transaction, so a failure leaves the folder archived and the whole restore retryable.

The tradeoff, stated plainly: both orderings now allow a transient window where a folder and its contents disagree — the inverse of what I originally optimized for ("a concurrent reader never sees an active resource inside a folder that has vanished"). That was the wrong thing to optimize for. The window is transient and self-healing on retry; the alternative silently and permanently strands data. The reasoning is written into the TSDoc on archiveFolderCascade and restoreFolder so it doesn't get "fixed" back.

Also in this round: the folder DELETE handler now rethrows HttpError instead of flattening it to a 500. deleteTable can still raise a 423 TableLockedError if a lock lands in the window between the subtree guard and the archive, and withRouteHandler already maps HttpError.statusCode correctly — the route's own catch was swallowing it.

Review round 3 — two follow-ons from the reordering

Nested stragglers were unreachable on retry (Greptile, P1). The delete subtree walk still collected active folders only. Because round 2 made the cascade stamp folders before children, a failure during the child pass leaves intermediate folders archived too — they then fail the IS NULL filter, drop out of the walk, and every resource beneath them becomes permanently unreachable. My round-2 reasoning ("children of the archived root are still found by parentId") only covered direct children of the root. The walk now admits folders that are active or already carry this cascade's own timestamp, which reaches the stragglers while still excluding folders archived independently under a different stamp. On a first delete no folder carries the new timestamp, so it is exactly equivalent to the old walk — the change only has effect on the retry path.

Locking was only half-confined to workflows (Bugbot, Medium). I gated assertFolderMutable but left the admin check and the locked write ungated — so an admin could persist locked: true on a kb/table folder (a column nothing reads for those types), and a non-admin update that merely mentioned locked was 403'd on a type where locking doesn't exist. This directly contradicted the invariant stated above, which is the giveaway that it belonged in the config rather than in scattered checks. Locking is now a declared capability (supportsLocking, set only on workflow), enforced at three levels: the routes reject the field outright with a 400, the admin gate and lock checks moved behind the capability, and the engine refuses to write it as a backstop for the copilot tools that bypass the route. This also removes the last three hardcoded resourceType === 'workflow' branches from the routes — that they survived round 1 is exactly how the gap got through.

Known follow-up (not fixed here, fails closed)

lib/resources/orchestration/restore-resource.ts dispatches its 'folder' arm to the workflow-bound performRestoreFolder, so it cannot restore an archived knowledge-base or table folder. It fails closed — a non-workflow folder id returns "Folder not found" rather than acting on the wrong tree — and the archived-resource listing that feeds it is itself resourceType-scoped, so no broken entry is surfaced today. POST /api/folders/[id]/restore already supports all three types. The KB/Tables follow-ups need to add arms to that union when they surface archived folders in their own recently-deleted lists.

Smaller decisions worth a line each

• file stays in FOLDER_RESOURCES but is refused at the boundary. The config is the complete data set for all four types; servedFolderResourceTypeSchema narrows the API to workflow | knowledge_base | table. File folders still write the legacy workspace_file_folders table — that cutover is separate and still pending, and accepting file here would point the API at the wrong storage.
• /api/folders/[id]/duplicate stays workflow-only. It duplicates the workflows contained in the folder, which has no kb/table analogue. Untouched, and it still filters resourceType = 'workflow'.
• resourceType is required on the id-keyed routes (as a query param on PUT/DELETE /api/folders/[id]). Folder ids are UUIDs and cannot collide, but without the filter a caller holding a knowledge-base folder id could drive it through the workflow surface. Every folder query in the PR carries its resourceType filter, including id-keyed lookups.
• One widened cleanup predicate, not one target per type. background/cleanup-soft-deletes.ts now hard-deletes folder rows for workflow | knowledge_base | table via a single inArray — same table, same soft-delete column, same workspace scoping, so splitting it would only multiply the batched scans. file stays excluded so the pass never touches rows it does not own.

Type of Change

• New feature
• Bug fix
• Breaking change
• Documentation
• Other: internal refactor + one deliberate behavior change (see above)

Testing

New: apps/sim/lib/folders/cascade.test.ts (25 tests). The folder engine that preceded this one shipped with no cascade tests at all — that was the single biggest gap and it is closed here. Coverage:

• Subtree resolution for both the active (delete) and archived (restore) directions, including that a parent cycle terminates the walk instead of recursing forever.
• Timestamp isolation — every cascade statement is asserted to carry the folder's own soft-delete timestamp, so independently-deleted siblings are provably not swept in or revived.
• archiveChildren delegation: the hook receives the resolved subtree and the child table is never written directly.
• Dependent restore keyed by the restored child ids, and skipped entirely when nothing was restored.
• An explicit "fixed statement count regardless of subtree depth" assertion over a six-folder subtree — this is the regression guard for the N+1 restore.
• FOLDER_RESOURCES invariants: one entry per type, each keyed consistently with its own resourceType, distinct cascade count keys, and every buildSoftDeleteSet provably writing the property named by its own deletedKey.

Plus route tests for the reorder 409 and for a delete-locked resource surfacing as 423, unit tests on folderMutationStatus, a restoreFolderRows timestamp test, two FOLDER_RESOURCES invariants added in review round 1 (every archiveChildren has a restore counterpart; tables declare a delete guard), and two ordering tests added in round 2 — one asserting the folder UPDATE precedes the child UPDATE, and one asserting the folder stamp is already in place at the moment an archiveChildren hook is invoked, which is the property that actually makes the retry safe.

Gates: bunx tsc --noEmit clean in apps/sim and packages/testing; bun run lint, check:api-validation, check:migrations, check:react-query, check:client-boundary all pass. Vitest: 386 files / 4784 tests green across lib/folders, app/api/folders, lib/workflows, lib/knowledge, lib/table, hooks, app/workspace, background, lib/copilot, lib/resources, app/api/knowledge, app/api/table.

Reviewers should focus on: the restore behavior change above; the FOLDER_RESOURCES entries read as a schema diff (columns, deletedKey vs buildSoftDeleteSet, membership scope); and that no folder query lost its resourceType filter.

I also ran a separate sweep of every query against the folder table across the repo — routes, copilot/VFS, logs, forking, admin v1, background — checking each for its resourceType constraint, since kb/table rows becoming real turns a previously-harmless missing filter into a cross-type leak. All scoped; the only unconstrained write is a name update already pinned by primary key inside a scoped transaction.

No migration — #6014 already added the folder table, the resourceType enum, and the folder_id columns on knowledge_base and user_table_definitions. Those columns are all-NULL today; this PR is the first writer. KB and Tables folders are greenfield: no legacy table, no existing rows, nothing to backfill.

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)

Screenshots/Videos

No UI in this PR — it is the shared backend layer. The KB and Tables folder UIs land in the two follow-ups.

[vercel]

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 2:06am

[waleedlatif1]

@greptile

[cursor]

PR Summary

High Risk
Large refactor of folder delete/restore/cascade semantics across workflows, knowledge bases, and tables, including a deliberate change to workflow dependent restore behavior and new failure modes (423 locks, 409 conflicts).

Overview
Introduces a generic folder engine (lib/folders/config, cascade, lifecycle) and moves workflow-only orchestration (~620 lines) to thin wrappers that pass resourceType: 'workflow'. Folder APIs and React Query now take resourceType (workflow | knowledge_base | table; file excluded at the contract) on list/create/update/delete/restore/reorder, with per-type query cache keys so trees do not overwrite each other.

Workflow-only locking is gated via supportsLocking on the config (routes return 400 for locked on KB/table; assertFolderMutable only when locking applies). Delete/restore cascades use one shared timestamp, delegate KB/table/workflow child work to canonical services (not bare row updates), enforce delete-locked tables as 423, and fix retry ordering (folders stamped before children on delete; hook restores before folder rows on restore). Reorder reparent sibling name clashes map to 409 instead of 500; cycle checks are wouldCreateFolderCycle(..., resourceType).

Deliberate workflow restore change: dependents (schedules, webhooks, etc.) are restored only when their archivedAt matches the folder cascade timestamp—user-disabled dependents are no longer silently re-enabled. Soft-delete cleanup widens to workflow | knowledge_base | table folder rows.

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

[waleedlatif1]

@cursor review

[greptile-apps]

Greptile Summary

Adds a shared resourceType-driven folder engine (config, cascade, lifecycle) so workflow, knowledge_base, and table folders share one create/update/delete/restore path, with thin workflow wrappers and resourceType-aware API/hooks.

• Introduces FOLDER_RESOURCES config plus cascade/lifecycle modules for generic folder ops
• Threads resourceType through folder API routes, contracts, React Query keys, and cleanup
• Delegates workflow/KB/table child archive/restore to canonical services with shared timestamps
• Fixes cascade ordering and subtree collection so partial delete/restore failures stay retryable

Confidence Score: 5/5

Safe to merge; prior cascade timestamp and nested-straggler issues are fixed and no remaining blocking failures were identified on this follow-up pass.

Prior cascade re-delete stamp desync and nested-straggler retry gaps are addressed in the current delete/collect path; no blocking failure remains for this PR.

Important Files Changed

Filename	Overview
apps/sim/lib/folders/cascade.ts	Shared cascade stamps folders before children, collects retry subtrees by active-or-same-timestamp, and restores only matching timestamps.
apps/sim/lib/folders/lifecycle.ts	deleteFolder reuses existing deletedAt; restoreFolder runs hook children before clearing folder tombs so partial failures stay retryable.
apps/sim/lib/folders/config.ts	Per-resource config wires workflow/KB/table archive and restore hooks, delete guards, and locking capability.
apps/sim/lib/folders/cascade.test.ts	Covers subtree retry admission, stamp ordering, timestamp isolation, and FOLDER_RESOURCES pairing invariants.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  API["Folder API routes"] --> Life["lifecycle.ts"]
  Life --> Config["FOLDER_RESOURCES"]
  Life --> Cascade["cascade.ts"]
  Cascade --> Folders["folder rows + shared timestamp"]
  Cascade -->|default path| ChildUpdate["child / dependent UPDATEs"]
  Cascade -->|hooks| Canonical["archive/restore via KB/table/workflow services"]

Loading

_{Reviews (5): Last reviewed commit: "fix(folders): route the delete lock chec..." | Re-trigger Greptile}

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[waleedlatif1]

@greptile

[waleedlatif1]

@cursor review

[waleedlatif1]

@greptile

[waleedlatif1]

@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 bda9fc5. Configure here.}