fix(files): keep folder creation working under a slash-named parent

Author: mzxchandra

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/file-upload/file-upload.tsx

@@ -18,6 +18,7 @@ import { parseWorkspaceFileFolderDisplayPath } from '@/lib/workspace-files/folde
 import { isFileInFolderScope } from '@/lib/workspace-files/folder-path-selection'
 import { formatDisplayText } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/formatted-text'
 import { getWorkflowSearchLabelHighlight } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/workflow-search-highlight'
+import { readFolderPath } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/sim-folder-tree-selector/selection'
 import { useSubBlockValue } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/hooks/use-sub-block-value'
 import { useActiveSearchTarget } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/providers/active-search-target-provider'
 import {
@@ -341,8 +342,13 @@ export function FileUpload({
     blockId,
     folderScope?.recursiveFieldId ?? subBlockId
   )
-  const folderScopePath =
-    folderScope && typeof folderScopeValue === 'string' ? folderScopeValue.trim() : ''
+  /*
+   * Through `readFolderPath` rather than a `typeof === 'string'` check: a value
+   * saved by the multi-select tree that preceded this one is a JSON array, and
+   * its literal text passes a string check and then matches no folder, so every
+   * file is filtered out of a picker that looks correctly configured.
+   */
+  const folderScopePath = folderScope ? readFolderPath(folderScopeValue) : ''
   const folderScopeIncludesSubfolders =
     !folderScope?.recursiveFieldId ||
     folderScopeRecursive === undefined ||

apps/sim/lib/api/contracts/tools/file.ts

@@ -60,7 +60,11 @@ export const fileManageAppendBodySchema = z.object({
   /**
    * Folder the name is resolved inside. A name is only unique within a folder,
    * so without this a duplicate name resolves to the oldest match anywhere.
-   * Ignored when `fileName` is already a canonical id.
+   *
+   * It constrains a canonical id too, rather than being ignored for one: an id
+   * that does not sit in the named folder is a `404`. Refusing is the safer
+   * reading of a contradictory request — silently preferring the id would write
+   * to a file outside the folder the caller named.
    */
   folderPath: v2FolderPathInputSchema.optional(),
   includeSubfolders: z.boolean().optional(),

apps/sim/lib/workspace-files/application/workspace-file-folders.test.ts

@@ -61,6 +61,7 @@ vi.mock('@sim/audit', () => ({
 }))
 vi.mock('@/lib/realtime/notify', () => ({ notifyWorkspaceFilesChanged: mockNotify }))
 
+import { OrchestrationError } from '@/lib/core/orchestration/types'
 import {
   createWorkspaceFileFolderOperation,
   deleteWorkspaceFileFolderOperation,
@@ -103,30 +104,70 @@ describe('workspace file folder operations', () => {
     mockArchive.mockResolvedValue({ files: 0, folders: 1, fileIds: [], folderIds: ['folder-1'] })
   })
 
+  const LEAF = {
+    folder: {
+      id: 'folder-c',
+      name: 'C',
+      path: 'A/B/C',
+      createdAt: new Date(),
+      updatedAt: new Date(),
+    },
+    path: 'A/B/C',
+  }
+
   /*
    * The tool description promises "Parent folders are created as needed", and
-   * the manager throws "Parent folder not found" when they are not — so the
-   * ancestors are materialized first, the same way the write path materializes
-   * its destination. Only the ancestors: the leaf keeps its own call so it
-   * still audits and still conflicts when something is already there.
+   * the manager throws "Parent folder not found" when they are not — so a
+   * missing ancestor is materialized and the create retried. Only the
+   * ancestors: the leaf keeps its own call so it still audits and still
+   * conflicts when something is already there.
    */
-  it('materializes missing ancestors before creating the leaf', async () => {
+  it('materializes missing ancestors and retries the leaf', async () => {
     mockEnsure.mockResolvedValue({
       folderId: 'folder-b',
       createdFolderIds: ['folder-a', 'folder-b'],
     })
-    mockCreate.mockResolvedValue({
-      folder: { id: 'folder-c', name: 'C', path: 'A/B/C', createdAt: new Date(), updatedAt: new Date() },
-      path: 'A/B/C',
-    })
+    mockCreate
+      .mockRejectedValueOnce(new OrchestrationError('not_found', 'Parent folder not found'))
+      .mockResolvedValue(LEAF)
 
     await createWorkspaceFileFolderOperation.execute({
       principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' },
       input: { workspaceId: 'ws-1', path: '/A/B/C' },
     })
 
     expect(mockEnsure).toHaveBeenCalledWith(expect.objectContaining({ pathSegments: ['A', 'B'] }))
-    expect(mockCreate).toHaveBeenCalledWith(expect.objectContaining({ path: '/A/B/C' }))
+    expect(mockCreate).toHaveBeenCalledTimes(2)
+  })
+
+  /*
+   * A folder name may contain a slash; a path segment may not. Materializing
+   * ancestors up front re-normalized the decoded name and rejected the folder's
+   * own existing parent, so the ancestors are only touched once the create has
+   * actually reported the parent missing.
+   */
+  it('does not touch the materializer when the parent already exists', async () => {
+    mockCreate.mockResolvedValue(LEAF)
+
+    await createWorkspaceFileFolderOperation.execute({
+      principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' },
+      input: { workspaceId: 'ws-1', path: '/A/Q3%2FQ4/C' },
+    })
+
+    expect(mockEnsure).not.toHaveBeenCalled()
+    expect(mockCreate).toHaveBeenCalledTimes(1)
+  })
+
+  it('surfaces a create failure that is not a missing parent', async () => {
+    mockCreate.mockRejectedValue(new OrchestrationError('conflict', 'Folder already exists'))
+
+    await expect(
+      createWorkspaceFileFolderOperation.execute({
+        principal: { kind: 'session', userId: 'user-1', sessionId: 'session-1' },
+        input: { workspaceId: 'ws-1', path: '/A/B/C' },
+      })
+    ).rejects.toThrow(/already exists/)
+    expect(mockEnsure).not.toHaveBeenCalled()
   })
 
   it('does not materialize anything for a top-level folder', async () => {

apps/sim/lib/workspace-files/application/workspace-file-folders.ts

@@ -165,6 +165,45 @@ async function executeListWorkspaceFileFolders(args: {
   return { folders }
 }
 
+/**
+ * Creates a folder at a path, materializing missing ancestors.
+ *
+ * Ancestors are created only after the direct attempt reports the parent
+ * missing, rather than up front. `ensureWorkspaceFileFolderPath` re-normalizes
+ * every segment it is handed, and a folder name is allowed to contain a slash
+ * while a segment is not — so materializing first rejected `/A/Q3%2FQ4/C` on
+ * its own existing parent. Reaching for the materializer only when the parent
+ * is genuinely absent keeps that path untouched in the common case.
+ *
+ * Only the ancestors go through the materializer. The leaf keeps its own call
+ * so it still emits FOLDER_CREATED with a full record, and still conflicts when
+ * something is already there — the materializer is silent on both counts, so
+ * routing the whole path through it would make "create" stop meaning create.
+ */
+async function createWorkspaceFileFolderAtPathCreatingAncestors(params: {
+  workspaceId: string
+  userId: string
+  path: string
+}) {
+  try {
+    return await createWorkspaceFileFolderAtPath(params)
+  } catch (error) {
+    const parentMissing =
+      error instanceof OrchestrationError &&
+      error.code === 'not_found' &&
+      error.message === 'Parent folder not found'
+    const segments = parseFolderPath(params.path)
+    if (!parentMissing || segments.length <= 1) throw error
+
+    await ensureWorkspaceFileFolderPath({
+      workspaceId: params.workspaceId,
+      userId: params.userId,
+      pathSegments: segments.slice(0, -1),
+    })
+    return await createWorkspaceFileFolderAtPath(params)
+  }
+}
+
 async function executeCreateWorkspaceFileFolder(args: {
   principal: Parameters<typeof resolvePrincipalAttribution>[0]
   input: CreateWorkspaceFileFolderInput
@@ -173,31 +212,9 @@ async function executeCreateWorkspaceFileFolder(args: {
   const attribution = resolvePrincipalAttribution(args.principal, {
     workspaceBillingOwnerUserId: args.context.billedAccountUserId,
   })
-  if (args.input.path !== undefined) {
-    /*
-     * Materialize the ancestors before creating the leaf, so creating
-     * `/A/B/C` works when `/A/B` does not exist yet — which is what the tool
-     * description promises and what the write path already does for its own
-     * destination.
-     *
-     * Only the ancestors go through the materializer. The leaf keeps its
-     * existing call so it still emits FOLDER_CREATED with a full record, and
-     * still conflicts when something is already there — the materializer is
-     * silent on both counts, so routing the whole path through it would make
-     * "create" stop meaning create.
-     */
-    const segments = parseFolderPath(args.input.path)
-    if (segments.length > 1) {
-      await ensureWorkspaceFileFolderPath({
-        workspaceId: args.context.workspaceId,
-        userId: attribution.attributedUserId,
-        pathSegments: segments.slice(0, -1),
-      })
-    }
-  }
   const result =
     args.input.path !== undefined
-      ? await createWorkspaceFileFolderAtPath({
+      ? await createWorkspaceFileFolderAtPathCreatingAncestors({
           workspaceId: args.context.workspaceId,
           userId: attribution.attributedUserId,
           path: args.input.path,