feat(files): folder operations for the File block, and folders as scope - #7377
feat(files): folder operations for the File block, and folders as scope#7377mzxchandra wants to merge 8 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Greptile SummaryAdds folder-aware file operations across the File block, agent tools, runtime execution, and workflow editor.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/blocks/blocks/file.ts | Extends File block configuration with folder operations and folder-scoped inputs; the previously reported helper type issue is addressed with a dedicated interface. |
| apps/sim/lib/internal/file/operations.ts | Dispatches the expanded file and folder operation set and resolves folder scopes for existing file operations. |
| apps/sim/lib/workspace-files/application/workspace-file-folders.ts | Implements application-level folder creation, movement, deletion, restoration, and path resolution. |
| apps/sim/lib/workspace-files/directory-listing.ts | Builds bounded mixed file-and-folder listings with recursive depth and search filtering. |
| apps/sim/lib/workspace-files/folder-path-selection.ts | Provides canonical path parsing and folder-scope membership logic, including escaped slash handling. |
| apps/sim/tools/file/folders.ts | Defines the new agent-callable folder and file-move tool surfaces. |
| 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 workflow editor’s folder-tree selection interface. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
U[Workflow editor or agent] --> B[File block and file tools]
B --> P[Canonical folder-path resolution]
P --> O{Requested operation}
O -->|Read or compress| S[Expand folder scope to files]
O -->|List| L[List folders and files]
O -->|Create, move, delete, restore| F[Folder application operations]
O -->|Write, append, move file| W[File application operations]
S --> R[Workspace file storage]
L --> R
F --> R
W --> R
Reviews (4): Last reviewed commit: "fix(files): import the folder picker fro..." | Re-trigger Greptile
There was a problem hiding this comment.
2 issues found across 42 files
Confidence score: 4/5
apps/sim/tools/file/folders.tsrejects direct list requests when optionaldepthorlimitisnullor blank, while explicit0must remain valid; normalize only blank values toundefinedbefore validation.sim-folder-tree-selector.tsxattaches the selected-folder clear handler to anaria-hiddenSVG, leaving keyboard and screen-reader users without the visible clear action; move the action to an accessible control outside the trigger button.
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/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/sim-folder-tree-selector/sim-folder-tree-selector.tsx">
<violation number="1" location="apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/sim-folder-tree-selector/sim-folder-tree-selector.tsx:122">
P3: When a selected folder is cleared, keyboard and screen-reader users cannot invoke the visible clear affordance because the handler is attached to an aria-hidden SVG. Move the clear action outside the trigger button into a separate focusable control, or provide equivalent keyboard semantics.</violation>
</file>
<file name="apps/sim/tools/file/folders.ts">
<violation number="1" location="apps/sim/tools/file/folders.ts:129">
P2: When a direct tool caller supplies `null` or `''` for optional `depth` or `limit`, the list request is rejected before listing. Normalize blank values to `undefined` for both fields while preserving explicit numeric `0`.</violation>
</file>
Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.
Re-trigger cubic
Adds folder operations to file_v5 and to 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. Its entries are a discriminated union on kind, so a consumer narrows before reaching for the fields only one side has, and the listing is capped with a truncated flag rather than being 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, and 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: a folder is not something you can append to, so it shapes the options and does not travel. Write gains a folder destination, placed above File Name because it names where before it names what. Every folder field is one single-select tree control paired with a manual entry. The picker cannot hold a reference expression — `<` autocomplete comes from TagDropdown, which scans for the last `<` before the cursor and so only exists on a text surface — which is why the pair exists rather than being a convenience. Root is the absence of a selection rather than a row, since a root row would be meaningless on create and wrong on delete. Path handling is the part most worth reviewing. 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 "/", so folderPathSegments decodes by the leading slash the canonical form carries, and resolveFolderIdsForPaths, isFileInFolderScope and selectDirectoryEntries are pure and tested against exactly that case. Delete Folder keeps its recursive flag as a guard rather than 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.
The list operation's entry selection had no tests. These pin the parts that are easy to get subtly wrong: a file counts one level below its folder, so the deepest folder a depth admits arrives without its contents; search filters the result rather than the traversal, so a deep match still reports even when its parent folders do not match; and a cut listing reports truncated rather than looking complete.
e3fb656 to
b5a77e3
Compare
Three fixes from the AI reviewers on #7377. The single-file picker kept its unscoped options when the folder changed: singleFileOptions read scopedWorkspaceFiles but listed workspaceFiles in its memo dependencies, so it never recomputed. That is the picker Append uses, so the folder narrowing I added for it did nothing after the first render. The v2 files-folders route called toWorkspaceFileFolderPathView while importing staging's toV2Folder — a conflict resolution I got half right, and a compile error I would have found by running the type-check I skipped. The file is back to staging's version, since keeping its projection was the decision anyway. fileFamilyInput took Record<string, any>, which the repo's TypeScript rules forbid. It now takes an interface naming the three fields it actually reads, so a shape change fails at the call site rather than at run time.
There was a problem hiding this comment.
All reported issues were addressed
Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.
Re-trigger cubic
Three P1s from cubic, all real, all mine, and all introduced by the rebase. Folder expansion was gone from read, get content and compress. Taking the upstream copy of the operations file and re-applying only the folder CASES dropped expandFolderPathsToFileIds and the folderPaths handling with it, so the block sent a folder, the contract accepted it, the tool forwarded it, and the operation ignored it — a folder-only selection failed with "File is required". file_write never read folderPath: it derived the destination from the file name alone, so choosing a folder wrote to the workspace root. It now prefixes the picked folder's decoded segments onto whatever the name itself spells. The six folder tool ids were registered in FILE_MANAGE_TOOL_IDS, which chooses the handler branch, but not in FILE_TOOL_IDS, which decides whether the internal registry has a handler at all. Every workflow execution of them would have failed before reaching the branch. None of this was caught because the tests covered the two ends — the block's params and the pure path helpers — and nothing covered the middle. Worse, the test file's module mock stubbed one export from workspace-file-folders, so the five folder operations were undefined and no test could reach a folder case at all. The mock is complete now, and five tests cover the wiring: a folder-only read expands, a missing folder refuses rather than reading nothing, write lands in the chosen folder and still lands at the root without one, and list returns folders and files together.
…lock Two more from cubic, both correct. The workflow diff rendered every folder change as the same generic label. Adding sim-folder-tree-selector to SELECTOR_TYPES_HYDRATION_REQUIRED without a resolver makes each consumer of that list fall back to a semantic placeholder — the canvas card showed "-" for the same reason, which I fixed there and not here. The comparison path now calls the same resolveFolderPathLabel, so a diff between two folders shows which two. Max Depth and Limit were bounded only at the contract, so an out-of-range value was accepted while configuring and failed on the first run. They now carry the same ceilings in the block, where the mistake is made.
|
@greptile review |
|
@cubic review |
@mzxchandra I can't start this review because your workspace has reached its monthly review limit. cubic has reviewed 574,058 of the 560,000 allowed lines of code this month. Reviews resume on 28 September 2026 (in 27 days). Enable flex capacity to cover overages automatically and resume reviews now. Learn how flex capacity works. To help optimise your usage, you can tune cubic to get the most out of your usage limits:
|
…tools # Conflicts: # apps/sim/tools/generated/tool-metadata.ts
|
@cubic review |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 43 files
Confidence score: 2/5
apps/sim/blocks/blocks/file.ts: AdvancedAppendcan target a same-named file outside the selected folder becausefolderPathis not forwarded, risking writes to the wrong file — pass the selected folder scope to the tool.apps/sim/lib/internal/file/operations.ts: Overwrite fails to find existing files when folder names contain/, so updates may create duplicates instead of replacing the intended file — resolve the overwrite target by folder ID.
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/blocks/blocks/file.ts">
<violation number="1" location="apps/sim/blocks/blocks/file.ts:1153">
P1: When Append uses advanced mode, `appendFileName` supplies only a filename, but this scope is not forwarded to the tool, so duplicate names can append to a file outside the selected folder. Forward `folderPath` for manual filename appends while retaining ID-based picker resolution.</violation>
</file>
<file name="apps/sim/lib/internal/file/operations.ts">
<violation number="1" location="apps/sim/lib/internal/file/operations.ts:997">
P2: When `overwrite` is enabled for a folder whose name contains `/`, the existing file is not found because the decoded folder segment is reinterpreted as two path segments. Resolve the overwrite target by folder ID and leaf name, or encode each reference segment before lookup.</violation>
</file>
| acceptedTypes: '*', | ||
| placeholder: 'Select workspace files', | ||
| multiple: true, | ||
| folderScope: FOLDER_SCOPE, |
There was a problem hiding this comment.
P1: When Append uses advanced mode, appendFileName supplies only a filename, but this scope is not forwarded to the tool, so duplicate names can append to a file outside the selected folder. Forward folderPath for manual filename appends while retaining ID-based picker resolution.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/sim/blocks/blocks/file.ts, line 1153:
<comment>When Append uses advanced mode, `appendFileName` supplies only a filename, but this scope is not forwarded to the tool, so duplicate names can append to a file outside the selected folder. Forward `folderPath` for manual filename appends while retaining ID-based picker resolution.</comment>
<file context>
@@ -973,9 +1150,9 @@ export const FileV5Block: BlockConfig<FileParserV3Output> = {
acceptedTypes: '*',
placeholder: 'Select workspace files',
multiple: true,
+ folderScope: FOLDER_SCOPE,
mode: 'basic',
condition: { field: 'operation', value: 'file_read' },
</file context>
| * decoded to names here — the same names `splitWorkspaceFilePath` yields | ||
| * — because the folder operation takes decoded segments. | ||
| */ | ||
| const folderSegments = folderPath |
There was a problem hiding this comment.
P2: When overwrite is enabled for a folder whose name contains /, the existing file is not found because the decoded folder segment is reinterpreted as two path segments. Resolve the overwrite target by folder ID and leaf name, or encode each reference segment before lookup.
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 997:
<comment>When `overwrite` is enabled for a folder whose name contains `/`, the existing file is not found because the decoded folder segment is reinterpreted as two path segments. Resolve the overwrite target by folder ID and leaf name, or encode each reference segment before lookup.</comment>
<file context>
@@ -908,7 +987,16 @@ export async function executeFileManageOperation(
+ * decoded to names here — the same names `splitWorkspaceFilePath` yields
+ * — because the folder operation takes decoded segments.
+ */
+ const folderSegments = folderPath
+ ? [...parseFolderPath(folderPath), ...nameSegments]
+ : nameSegments
</file context>
CI's Lint and Test failed on sub-block-password.test.tsx, a test this branch does not touch, with a missing icon export from a partial mock. The cause was a deep import. sub-block.tsx takes every other subblock component from the components barrel, and that test mocks the barrel wholesale — which is what keeps the editor's module graph out of its way. My folder picker was imported directly from its file instead, bypassing the mock and pulling the real chain in behind it: use-resource-folders, the realtime invalidation room, the socket provider, the workflow stores, and through them the block and trigger registries. Every trigger's icon then had to appear in that test's icon mock. Exporting the picker from the barrel and importing it from there fixes it with no change to the test, which is the sign it was the right fix — I had first patched the mock, then stubbed the socket provider, and both were treating a symptom. The repo's import rules already say to use the barrel; I had not.
|
@cubic review |
@mzxchandra I have started the AI code review. It will take a few minutes to complete. |
|
@greptile review |
What
Adds folder operations to
file_v5and to 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, the whole subtree under Recursive, subject to Max Depth and Search. Entries are a discriminated union on
kind, so a consumer narrows before reaching for the fields only one side has. 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 when the workflow runs so a file added later is included. Append only narrows — a folder is not something you can append to, so it shapes the options and does not travel.
Write gains a folder destination, above File Name, because it names where before it names what.
Where to spend review attention
Path handling — this is where the bugs live. 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 the canonical form always carries;resolveFolderIdsForPaths,isFileInFolderScopeandselectDirectoryEntriesare pure and tested against exactly that case. I shipped this bug once during development — it surfaced as "Workspace file folder path contains an empty name" on a correctly picked folder.Delete Folder's recursive flag is a guard, not a scope. Without it, deleting a non-empty folder fails, and it is
user-onlyso a model asked to "clean up" a folder cannot set it on a guess. Deliberately unlike the read family's Include Subfolders, which is a scope and defaults on.Canonical pairs. Every folder field is a single-select tree paired with a manual 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. A checkbox tree has no cursor.No new tools for folder-scoped reads. The manage operation already accepts
folderPathsalongside file ids, so the folder scope reusesfile_read/file_get_content/file_compress. Two nearly-overlapping tools are harder for an agent to choose between than one that takes either.Root is the absence of a selection, not a row — a root row would be meaningless on create and wrong on delete. The placeholders say so.
Known characteristic, not a blocker
The
listcase reads the workspace's full folder and file lists and filters in memory, the same wayexpandFolderPathsToFileIdsand roughly ten existing callers oflistAllWorkspaceFilesdo. Output is capped bylimitwith atruncatedflag, but the read itself is not bounded.queryWorkspaceFilePageis the paged, folder-scoped alternative if this becomes a problem on large workspaces — worth doing as a follow-up across all callers rather than only this one.Testing
blocks,tools/file,lib/internal/file,lib/workspace-files,lib/workflows/subblocks.check-canvas-sentences, generated docs, and tool metadata.check:api-validationandcheck:client-boundaryclean; biome clean.folder-path-selection,directory-listing,display) has direct unit coverage including the slash-in-name case.Follow-ups (not in this PR)