feat(files): folder operations for the File block, and folders as scope - #7388
Open
mzxchandra wants to merge 4 commits into
Open
feat(files): folder operations for the File block, and folders as scope#7388mzxchandra wants to merge 4 commits into
mzxchandra wants to merge 4 commits into
Conversation
Adds folder operations to file_v5 and the agent tool surface, and makes a folder a scope on the file operations that already existed rather than a second set of operations beside them. New operations: List, Create Folder, Move Folder, Delete Folder, Restore Folder, and Move File. List answers "what is in here" — subfolders and files together, direct children by default, the whole subtree under Recursive, subject to Max Depth and Search. Entries are a discriminated union on kind, and the listing is capped with a truncated flag rather than unbounded now that it includes files. Read, Get Content, Compress and Append gain an optional Folder above their file picker. It narrows what the picker offers; on the three read operations it also stands for that folder's files when none are picked, resolved when the workflow runs so a file added later is included. Append only narrows the picker, but the folder does travel when the advanced entry supplies a name rather than an id, because a name is only unique inside a folder. Write gains a folder destination, placed above File Name because it names where before it names what. Two things are worth a reviewer's attention. Path handling. Two spellings circulate: the stored display path, which backslash-escapes a slash inside a folder name, and the canonical percent-encoded path the tools take. A folder genuinely named "Q3/Q4" is one level in both and two if either is split on "/". folderPathSegments picks the parser by the leading slash the canonical form always carries, and resolveFolderIdsForPaths, isFileInFolderScope and selectDirectoryEntries are pure and tested against exactly that case. Where a folder is known, the code resolves by id and never builds a path-shaped reference at all. Delete Folder's recursive flag is a guard, not a scope. Without it, deleting a non-empty folder fails, and it is user-only so a model asked to clean up a folder cannot set it on a guess. This is deliberately the inverse of the read family's Include Subfolders, which is a scope and defaults on. Five file operations widen from copilot-only delegation to admit the executor, each justified by a tool in this change: files.list and files.folders.list for file_list, and files.folders.update / delete / restore for their tools. Principal kinds are unchanged and copilot keeps every operation it had.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
Greptile SummaryThe PR extends the File block and agent tool surface with folder listing and mutation operations, folder-scoped file operations, canonical folder-path handling, and corresponding editor controls and contracts.
Confidence Score: 4/5The PR appears safe to merge, with only a non-blocking type-safety cleanup needed in the new folder-operation tests. The folder operations, canonical path handling, execution wiring, authorization boundaries, and focused edge cases are aligned; the only accepted issue is that two test fixtures suppress contract checking through Files Needing Attention: apps/sim/blocks/blocks/file-folders.test.ts
|
| Filename | Overview |
|---|---|
| apps/sim/blocks/blocks/file.ts | Adds File block folder controls and maps all new folder and scoped-file operations into canonical tool parameters. |
| apps/sim/lib/internal/file/operations.ts | Implements runtime folder expansion, combined directory listing, folder mutations, and folder-aware write, append, compression, and file movement. |
| apps/sim/lib/workspace-files/application/workspace-file-folders.ts | Extends authorized folder use cases with canonical path addressing, recursive listing, and mutation result metadata. |
| apps/sim/lib/workspace-files/directory-listing.ts | Introduces bounded mixed folder/file subtree selection with depth, search, ordering, and truncation semantics. |
| apps/sim/lib/workspace-files/folder-path-selection.ts | Adds pure canonical/display path scope resolution used by runtime expansion and editor filtering. |
| apps/sim/tools/file/folders.ts | Defines the new folder and file-move agent tools and their schemas. |
| apps/sim/lib/api/contracts/tools/file.ts | Expands the shared File tool contract with canonical folder paths, list controls, folder mutations, and their result unions. |
| apps/sim/blocks/blocks/file-folders.test.ts | Provides broad wiring coverage for folder operations and path edge cases but introduces two explicit any casts contrary to repository type-safety rules. |
| apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/sim-folder-tree-selector/sim-folder-tree-selector.tsx | Adds the canonical folder-tree selector used by File block basic-mode fields. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
Editor[File block editor] --> Mapper[Operation parameter mapper]
Agent[Agent tool call] --> Contract[File tool contract]
Mapper --> Contract
Contract --> Executor[Internal file operation]
Executor --> Authz[Authorized workspace-file use case]
Authz --> Paths[Canonical folder path resolution]
Paths --> Folders[Folder manager]
Paths --> Files[Workspace file operations]
Folders --> Result[Structured tool result]
Files --> Result
Reviews (1): Last reviewed commit: "feat(files): folder operations for the F..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+367
to
+371
| * it wrong is silent. | ||
| */ | ||
| describe('the file pickers are scoped by the folder beside them', () => { | ||
| it.each(['readFile', 'getContentFile', 'compressFile', 'appendFile'])( | ||
| '%s follows the folder', |
Contributor
There was a problem hiding this comment.
The new folder-operation tests cast parameter fixtures to any, suppressing compile-time validation of the mapper's accepted shape and allowing the fixtures to drift from the block parameter contract without a type error.
Context Used: TypeScript conventions and type safety (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
6 issues found across 46 files
Confidence score: 3/5
file-upload.tsxpasses legacy JSON-array folder values directly toisFileInFolderScope, so valid selections can be rejected during render; normalize the value withreadFolderPathbefore applying the scope check.- Folder edge cases remain risky in
workspace-file-folders.tsandoperations.ts: escaped-slash parent folders can fail child creation, while/can incorrectly produceFolder not found; resolve decoded ancestors and handle the virtual root explicitly. file.tscan return 404 when a canonicalfileNameID is combined withfolderPath, because the lookup is still constrained by the folder; bypass folder expansion for canonical IDs or reject the combination.use-resource-folders.tscreates duplicate room listeners for multiple pickers, multiplying invalidations, whilefolder-display-path.tscan diverge from the v2 route projector; share the listener and replace the route utility with the common projector.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/sim/lib/workspace-files/folder-display-path.ts">
<violation number="1" location="apps/sim/lib/workspace-files/folder-display-path.ts:83">
P3: This projector duplicates `toV2Folder` while the v2 routes continue calling their copy. Replace the route utility with this shared projector, otherwise future path or invariant fixes can make v2 and tool responses diverge.</violation>
</file>
<file name="apps/sim/lib/workspace-files/application/workspace-file-folders.ts">
<violation number="1" location="apps/sim/lib/workspace-files/application/workspace-file-folders.ts:194">
P2: When creating a child below a folder whose name contains an escaped slash, this new ancestor materialization rejects the decoded parent name before the path creator resolves the existing folder. Make ancestor materialization path-segment aware, or skip it when the complete parent path already exists.</violation>
</file>
<file name="apps/sim/lib/api/contracts/tools/file.ts">
<violation number="1" location="apps/sim/lib/api/contracts/tools/file.ts:23">
P2: When `fileName` is a canonical file ID and `folderPath` is also supplied, append still constrains the lookup to that folder and can return 404. Make canonical IDs bypass folder expansion, or reject this combination instead of documenting that the folder is ignored.
(Based on your team's feedback about folder scope traveling only with named appends.)</violation>
</file>
<file name="apps/sim/lib/internal/file/operations.ts">
<violation number="1" location="apps/sim/lib/internal/file/operations.ts:637">
P2: When a caller supplies the valid root path `/` as a folder scope, expansion returns `Folder not found` instead of selecting root files. Handle the virtual root explicitly, including direct-root files when subfolders are disabled.</violation>
</file>
<file name="apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/file-upload/file-upload.tsx">
<violation number="1" location="apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/file-upload/file-upload.tsx:345">
P2: When a legacy folder selector value is stored as a JSON array, this passes the literal array text to `isFileInFolderScope`, which rejects it during render. Normalize the sibling value with `readFolderPath` before applying the scope.</violation>
</file>
<file name="apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-resource-folders.ts">
<violation number="1" location="apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-resource-folders.ts:75">
P2: When an editor renders multiple file folder pickers, each `useResourceFolders` invocation joins the same workspace-files room and registers its own changed listener. Every broadcast then repeats the same invalidations, so hoist this room subscription to one editor-level hook or add shared reference-counting.
(Based on your team's feedback about hoisting shared workspace room subscriptions.)</violation>
</file>
| folderScope?.recursiveFieldId ?? subBlockId | ||
| ) | ||
| const folderScopePath = | ||
| folderScope && typeof folderScopeValue === 'string' ? folderScopeValue.trim() : '' |
Contributor
There was a problem hiding this comment.
P2: When a legacy folder selector value is stored as a JSON array, this passes the literal array text to isFileInFolderScope, which rejects it during render. Normalize the sibling value with readFolderPath before applying the scope.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/file-upload/file-upload.tsx, line 345:
<comment>When a legacy folder selector value is stored as a JSON array, this passes the literal array text to `isFileInFolderScope`, which rejects it during render. Normalize the sibling value with `readFolderPath` before applying the scope.</comment>
<file context>
@@ -274,7 +326,40 @@ export function FileUpload({
+ folderScope?.recursiveFieldId ?? subBlockId
+ )
+ const folderScopePath =
+ folderScope && typeof folderScopeValue === 'string' ? folderScopeValue.trim() : ''
+ const folderScopeIncludesSubfolders =
+ !folderScope?.recursiveFieldId ||
</file context>
| const onFilesChanged = useCallback(() => { | ||
| if (workspaceId) invalidateWorkspaceFileBrowsers(queryClient, workspaceId) | ||
| }, [queryClient, workspaceId]) | ||
| useWorkspaceInvalidationRoom( |
Contributor
There was a problem hiding this comment.
P2: When an editor renders multiple file folder pickers, each useResourceFolders invocation joins the same workspace-files room and registers its own changed listener. Every broadcast then repeats the same invalidations, so hoist this room subscription to one editor-level hook or add shared reference-counting.
(Based on your team's feedback about hoisting shared workspace room subscriptions.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-resource-folders.ts, line 75:
<comment>When an editor renders multiple file folder pickers, each `useResourceFolders` invocation joins the same workspace-files room and registers its own changed listener. Every broadcast then repeats the same invalidations, so hoist this room subscription to one editor-level hook or add shared reference-counting.
(Based on your team's feedback about hoisting shared workspace room subscriptions.) </comment>
<file context>
@@ -0,0 +1,131 @@
+ const onFilesChanged = useCallback(() => {
+ if (workspaceId) invalidateWorkspaceFileBrowsers(queryClient, workspaceId)
+ }, [queryClient, workspaceId])
+ useWorkspaceInvalidationRoom(
+ isFileResource ? (workspaceId ?? '') : '',
+ ROOM_TYPES.WORKSPACE_FILES,
</file context>
| await ensureWorkspaceFileFolderPath({ | ||
| workspaceId: args.context.workspaceId, | ||
| userId: attribution.attributedUserId, | ||
| pathSegments: segments.slice(0, -1), |
Contributor
There was a problem hiding this comment.
P2: When creating a child below a folder whose name contains an escaped slash, this new ancestor materialization rejects the decoded parent name before the path creator resolves the existing folder. Make ancestor materialization path-segment aware, or skip it when the complete parent path already exists.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/workspace-files/application/workspace-file-folders.ts, line 194:
<comment>When creating a child below a folder whose name contains an escaped slash, this new ancestor materialization rejects the decoded parent name before the path creator resolves the existing folder. Make ancestor materialization path-segment aware, or skip it when the complete parent path already exists.</comment>
<file context>
@@ -154,6 +173,28 @@ async function executeCreateWorkspaceFileFolder(args: {
+ await ensureWorkspaceFileFolderPath({
+ workspaceId: args.context.workspaceId,
+ userId: attribution.attributedUserId,
+ pathSegments: segments.slice(0, -1),
+ })
+ }
</file context>
| operation: z.literal('write'), | ||
| workspaceId: z.string().min(1).optional(), | ||
| fileName: z.string().min(1).optional(), | ||
| folderPath: v2FolderPathInputSchema.optional(), |
Contributor
There was a problem hiding this comment.
P2: When fileName is a canonical file ID and folderPath is also supplied, append still constrains the lookup to that folder and can return 404. Make canonical IDs bypass folder expansion, or reject this combination instead of documenting that the folder is ignored.
(Based on your team's feedback about folder scope traveling only with named appends.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/api/contracts/tools/file.ts, line 23:
<comment>When `fileName` is a canonical file ID and `folderPath` is also supplied, append still constrains the lookup to that folder and can return 404. Make canonical IDs bypass folder expansion, or reject this combination instead of documenting that the folder is ignored.
(Based on your team's feedback about folder scope traveling only with named appends.) </comment>
<file context>
@@ -15,6 +20,7 @@ export const fileManageWriteBodySchema = z
operation: z.literal('write'),
workspaceId: z.string().min(1).optional(),
fileName: z.string().min(1).optional(),
+ folderPath: v2FolderPathInputSchema.optional(),
content: z.string().optional(),
/**
</file context>
| throw new OrchestrationError('not_found', `Folder not found: ${selection.missingPath}`) | ||
| } | ||
|
|
||
| return files.filter((file) => file.folderId && selection.folderIds.has(file.folderId)) |
Contributor
There was a problem hiding this comment.
P2: When a caller supplies the valid root path / as a folder scope, expansion returns Folder not found instead of selecting root files. Handle the virtual root explicitly, including direct-root files when subfolders are disabled.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/internal/file/operations.ts, line 637:
<comment>When a caller supplies the valid root path `/` as a folder scope, expansion returns `Folder not found` instead of selecting root files. Handle the virtual root explicitly, including direct-root files when subfolders are disabled.</comment>
<file context>
@@ -555,6 +592,60 @@ export function fileContentJsonResponse(
+ throw new OrchestrationError('not_found', `Folder not found: ${selection.missingPath}`)
+ }
+
+ return files.filter((file) => file.folderId && selection.folderIds.has(file.folderId))
+}
+
</file context>
| * Shared by the v2 route and the file tool surface so the two cannot disagree | ||
| * about what a folder's path is. | ||
| */ | ||
| export function toWorkspaceFileFolderPathView(folder: { |
Contributor
There was a problem hiding this comment.
P3: This projector duplicates toV2Folder while the v2 routes continue calling their copy. Replace the route utility with this shared projector, otherwise future path or invariant fixes can make v2 and tool responses diverge.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/lib/workspace-files/folder-display-path.ts, line 83:
<comment>This projector duplicates `toV2Folder` while the v2 routes continue calling their copy. Replace the route utility with this shared projector, otherwise future path or invariant fixes can make v2 and tool responses diverge.</comment>
<file context>
@@ -41,3 +43,59 @@ export function parseWorkspaceFileFolderDisplayPath(path: string): string[] {
+ * Shared by the v2 route and the file tool surface so the two cannot disagree
+ * about what a folder's path is.
+ */
+export function toWorkspaceFileFolderPathView(folder: {
+ name: string
+ path: string
</file context>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
…s.test.ts Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
This was referenced Sep 2, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #7377, which is the same change spread over 11 commits and two staging merges. Reviewers were reading a diff-of-diffs and finding issues in the fixes rather than in the change; this is one commit against current
staging.What
Adds folder operations to
file_v5and the agent tool surface, and makes a folder a scope on the file operations that already existed rather than a second set of operations beside them.New operations: List, Create Folder, Move Folder, Delete Folder, Restore Folder, Move File.
List answers "what is in here" — subfolders and files together, direct children by default, whole subtree under Recursive, subject to Max Depth and Search. Entries are a discriminated union on
kind; the listing is capped with atruncatedflag rather than unbounded now that it includes files.Read / Get Content / Compress / Append gain an optional Folder above their file picker. It narrows what the picker offers; on the three read operations it also stands for that folder's files when none are picked, resolved at run time so a file added later is included. Append only narrows the picker — but the folder does travel when the advanced entry supplies a name rather than an id, because a name is only unique inside a folder.
Write gains a folder destination, above File Name, because it names where before what.
Where to spend attention
Path handling. Two spellings circulate: the stored display path, which backslash-escapes a slash inside a folder name (
Reports/Q3\/Q4), and the canonical percent-encoded path the tools take (/Reports/Q3%2FQ4). A folder genuinely namedQ3/Q4is one level in both and two if either is split on/.folderPathSegmentspicks the parser by the leading slash;resolveFolderIdsForPaths,isFileInFolderScopeandselectDirectoryEntriesare pure and tested against that case. Where a folder is known, resolution goes by id and never builds a path-shaped reference.Delete's
recursiveis a guard, not a scope. Without it, deleting a non-empty folder fails, and it isuser-onlyso a model asked to "clean up" cannot set it on a guess. Deliberately the inverse of the read family's Include Subfolders, which is a scope and defaults on.The authz widening. Five operations move from copilot-only delegation to also admit the executor —
files.listandfiles.folders.listforfile_list, andfiles.folders.update/delete/restorefor their tools. Principal kinds are unchanged and copilot keeps everything it had; the only delta isdelegatedServicesgaining'executor', without which a workflow run cannot reach these at all.Canonical pairs. Every folder field is a single-select tree paired with a manual text entry. The pair is load-bearing, not a convenience:
<reference autocomplete comes fromTagDropdown, which scans for the last<before the cursor, so it can only live on a text surface. The one folder field that never travels (the picker-narrowing scope) deliberately has no pair.Known costs, already argued
listreads the workspace's full folder and file lists and filters in memory, matching ~10 existing callers oflistAllWorkspaceFiles. Output is capped withtruncated; the read is not.queryWorkspaceFilePageis the paged alternative, worth doing across all callers rather than only this one.Testing
tscclean; 4209 tests acrossblocks,tools/file,lib/internal/file,lib/workspace-files,lib/workflows; all 45 audits;check:api-validation;check:client-boundary; biome.Coverage is deliberately weighted at the wiring, because that is where this change broke repeatedly: a contract accepting a field is not the same as the operation reading it. Tests assert a folder-only read expands, a missing folder refuses rather than reading nothing, write lands in the chosen folder and still lands at root without one, an ambiguous name inside a scope refuses instead of picking, and a folder named
Q3/Q4survives both scope resolution and overwrite lookup.Browser end-to-end is in progress against this branch.