Skip to content

Commit 35198ea

Browse files
committed
fix(files): reconnect folder scope to the operation that runs it
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.
1 parent e889505 commit 35198ea

3 files changed

Lines changed: 267 additions & 6 deletions

File tree

‎apps/sim/lib/internal/file/operations.test.ts‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ const {
1111
mockDownloadFileFromStorage,
1212
mockDecompressArchiveBufferToWorkspaceFiles,
1313
mockEnsureWorkspaceFileFolderPath,
14+
mockListWorkspaceFileFolders,
15+
mockCreateWorkspaceFileFolder,
16+
mockUpdateWorkspaceFileFolder,
17+
mockDeleteWorkspaceFileFolder,
18+
mockRestoreWorkspaceFileFolder,
19+
mockListAllWorkspaceFiles,
1420
mockFetchWorkspaceFileBuffer,
1521
mockGetBoundWorkspaceFileSecretProvenance,
1622
mockLoadActiveWorkspaceContext,
@@ -29,6 +35,12 @@ const {
2935
mockDownloadFileFromStorage: vi.fn(),
3036
mockDecompressArchiveBufferToWorkspaceFiles: vi.fn(),
3137
mockEnsureWorkspaceFileFolderPath: vi.fn(),
38+
mockListWorkspaceFileFolders: vi.fn(),
39+
mockCreateWorkspaceFileFolder: vi.fn(),
40+
mockUpdateWorkspaceFileFolder: vi.fn(),
41+
mockDeleteWorkspaceFileFolder: vi.fn(),
42+
mockRestoreWorkspaceFileFolder: vi.fn(),
43+
mockListAllWorkspaceFiles: vi.fn(),
3244
mockFetchWorkspaceFileBuffer: vi.fn(),
3345
mockGetBoundWorkspaceFileSecretProvenance: vi.fn(),
3446
mockLoadActiveWorkspaceContext: vi.fn(),
@@ -107,6 +119,25 @@ vi.mock('@/lib/workspace-files/application/workspace-file-folders', () => ({
107119
ensureWorkspaceFileFolderPathOperation: {
108120
execute: (...args: unknown[]) => mockEnsureWorkspaceFileFolderPath(...args),
109121
},
122+
listWorkspaceFileFoldersOperation: {
123+
execute: (...args: unknown[]) => mockListWorkspaceFileFolders(...args),
124+
},
125+
createWorkspaceFileFolderOperation: {
126+
execute: (...args: unknown[]) => mockCreateWorkspaceFileFolder(...args),
127+
},
128+
updateWorkspaceFileFolderOperation: {
129+
execute: (...args: unknown[]) => mockUpdateWorkspaceFileFolder(...args),
130+
},
131+
deleteWorkspaceFileFolderOperation: {
132+
execute: (...args: unknown[]) => mockDeleteWorkspaceFileFolder(...args),
133+
},
134+
restoreWorkspaceFileFolderOperation: {
135+
execute: (...args: unknown[]) => mockRestoreWorkspaceFileFolder(...args),
136+
},
137+
}))
138+
139+
vi.mock('@/lib/workspace-files/application/list-workspace-files', () => ({
140+
listAllWorkspaceFiles: { execute: (...args: unknown[]) => mockListAllWorkspaceFiles(...args) },
110141
}))
111142

112143
vi.mock('@/lib/workspace-files/application/move-workspace-file-items', () => ({
@@ -245,6 +276,144 @@ function actorlessDeploymentPrincipal(workspaceId = 'workspace-1') {
245276
}
246277
}
247278

279+
/*
280+
* The folder path exists in three places — the block's params, the contract, and
281+
* the operation — and only the two ends were tested. A rebase dropped folder
282+
* expansion out of the middle, and every unit test stayed green while a
283+
* folder-only read failed with "File is required". These cover the middle.
284+
*/
285+
describe('file manage folder wiring', () => {
286+
const FOLDER_ROW = {
287+
id: 'folder-reports',
288+
parentId: null,
289+
name: 'Reports',
290+
path: 'Reports',
291+
createdAt: CONTENT_UPDATED_AT,
292+
updatedAt: CONTENT_UPDATED_AT,
293+
}
294+
295+
beforeEach(() => {
296+
vi.clearAllMocks()
297+
hybridAuthMockFns.mockCheckInternalAuth.mockResolvedValue({
298+
success: true,
299+
userId: 'user-1',
300+
authType: 'internal_jwt',
301+
})
302+
mockAssertActiveWorkspaceAccess.mockResolvedValue(undefined)
303+
mockResolveEffectiveWorkspacePermission.mockResolvedValue('write')
304+
mockLoadActiveWorkspaceContext.mockResolvedValue({
305+
workspaceId: 'workspace-1',
306+
workspaceOrganizationId: null,
307+
allowPersonalApiKeys: true,
308+
billedAccountUserId: 'user-1',
309+
})
310+
mockVerifyFileAccess.mockResolvedValue(true)
311+
mockGetWorkspaceFile.mockImplementation(async (_ws: string, fileId: string) =>
312+
workspaceFile(fileId)
313+
)
314+
mockLoadActiveWorkspaceFileContext.mockImplementation(async (fileId: string) => ({
315+
fileId,
316+
workspaceId: 'workspace-1',
317+
workspaceOrganizationId: null,
318+
allowPersonalApiKeys: true,
319+
billedAccountUserId: 'user-1',
320+
}))
321+
mockListWorkspaceFileFolders.mockResolvedValue({ folders: [FOLDER_ROW] })
322+
mockListAllWorkspaceFiles.mockResolvedValue({
323+
files: [
324+
{ ...workspaceFile('file-in-folder'), folderId: 'folder-reports' },
325+
{ ...workspaceFile('file-at-root'), folderId: null },
326+
],
327+
})
328+
mockEnsureWorkspaceFileFolderPath.mockImplementation(
329+
async ({ input }: { input: { pathSegments: string[] } }) => ({
330+
folderId: input.pathSegments.length === 0 ? null : 'folder-reports',
331+
createdFolderIds: [],
332+
})
333+
)
334+
mockUploadWorkspaceFile.mockResolvedValue({
335+
id: 'new-file',
336+
name: 'new.txt',
337+
key: 'workspace/workspace-1/new.txt',
338+
url: '/api/files/serve/new-file',
339+
})
340+
})
341+
342+
it('expands a folder-only read instead of rejecting it', async () => {
343+
const response = await POST(
344+
createMockRequest('POST', {
345+
operation: 'read',
346+
workspaceId: 'workspace-1',
347+
folderPaths: ['/Reports'],
348+
})
349+
)
350+
351+
// A folder alone is a complete selection, expanded when the workflow runs.
352+
expect(response.status).not.toBe(400)
353+
expect(mockListWorkspaceFileFolders).toHaveBeenCalled()
354+
expect(mockListAllWorkspaceFiles).toHaveBeenCalled()
355+
})
356+
357+
it('refuses a folder that does not exist rather than reading nothing', async () => {
358+
const response = await POST(
359+
createMockRequest('POST', {
360+
operation: 'read',
361+
workspaceId: 'workspace-1',
362+
folderPaths: ['/Nope'],
363+
})
364+
)
365+
const body = await response.json()
366+
367+
expect(body.success).toBe(false)
368+
expect(String(body.error)).toContain('Folder not found')
369+
})
370+
371+
it('writes into the folder it was given, not the workspace root', async () => {
372+
await POST(
373+
createMockRequest('POST', {
374+
operation: 'write',
375+
workspaceId: 'workspace-1',
376+
fileName: 'notes.md',
377+
folderPath: '/Reports',
378+
content: 'hello',
379+
})
380+
)
381+
382+
expect(mockEnsureWorkspaceFileFolderPath).toHaveBeenCalledWith(
383+
expect.objectContaining({ input: expect.objectContaining({ pathSegments: ['Reports'] }) })
384+
)
385+
})
386+
387+
it('still writes to the root when no folder is given', async () => {
388+
await POST(
389+
createMockRequest('POST', {
390+
operation: 'write',
391+
workspaceId: 'workspace-1',
392+
fileName: 'notes.md',
393+
content: 'hello',
394+
})
395+
)
396+
397+
expect(mockEnsureWorkspaceFileFolderPath).toHaveBeenCalledWith(
398+
expect.objectContaining({ input: expect.objectContaining({ pathSegments: [] }) })
399+
)
400+
})
401+
402+
it('lists what a folder holds, folders and files together', async () => {
403+
const response = await POST(
404+
createMockRequest('POST', { operation: 'list', workspaceId: 'workspace-1' })
405+
)
406+
const body = await response.json()
407+
408+
expect(body.success).toBe(true)
409+
expect(body.data.entries.map((entry: { name: string }) => entry.name)).toEqual([
410+
'Reports',
411+
'file-at-root.txt',
412+
])
413+
expect(body.data.truncated).toBe(false)
414+
})
415+
})
416+
248417
describe('file manage operations', () => {
249418
beforeEach(() => {
250419
vi.clearAllMocks()

‎apps/sim/lib/internal/file/operations.ts‎

Lines changed: 92 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {
2727
requestsPrivateToolMetadata,
2828
} from '@/lib/execution/private-tool-metadata'
2929
import { isSupportedFileType, parseBuffer } from '@/lib/file-parsers'
30-
import { buildFolderPath, ROOT_FOLDER_PATH } from '@/lib/folders/paths'
30+
import { buildFolderPath, parseFolderPath, ROOT_FOLDER_PATH } from '@/lib/folders/paths'
3131
import { ShareValidationError } from '@/lib/public-shares/share-manager'
3232
import {
3333
ArchiveError,
@@ -567,6 +567,53 @@ export function fileContentJsonResponse(
567567
)
568568
}
569569

570+
/**
571+
* Expands folder paths to the ids of every file beneath them.
572+
*
573+
* Resolution happens here, at run time, rather than when the block is
574+
* configured: picking a folder means "whatever is in it when this runs", so a
575+
* file added tomorrow is read tomorrow. Expanding in the picker would freeze a
576+
* snapshot instead.
577+
*
578+
* Path resolution itself lives in {@link resolveFolderIdsForPaths}; this is the
579+
* IO around it.
580+
*/
581+
async function expandFolderPathsToFileIds(args: {
582+
principal: Principal
583+
workspaceId: string
584+
folderPaths: string[] | undefined
585+
includeSubfolders: boolean | undefined
586+
}): Promise<string[]> {
587+
if (!args.folderPaths?.length) return []
588+
589+
const [{ folders }, { files }] = await Promise.all([
590+
listWorkspaceFileFoldersOperation.execute({
591+
principal: args.principal,
592+
input: { workspaceId: args.workspaceId },
593+
}),
594+
listAllWorkspaceFiles.execute({
595+
principal: args.principal,
596+
input: { workspaceId: args.workspaceId, scope: 'active' },
597+
}),
598+
])
599+
600+
const projected = folders.map((folder) => ({
601+
...toWorkspaceFileFolderPathView(folder),
602+
id: folder.id,
603+
parentId: folder.parentId,
604+
}))
605+
const selection = resolveFolderIdsForPaths(projected, args.folderPaths, {
606+
includeSubfolders: args.includeSubfolders,
607+
})
608+
if (selection.missingPath !== undefined) {
609+
throw new OrchestrationError('not_found', `Folder not found: ${selection.missingPath}`)
610+
}
611+
612+
return files
613+
.filter((file) => file.folderId && selection.folderIds.has(file.folderId))
614+
.map((file) => file.id)
615+
}
616+
570617
export async function executeFileManageOperation(
571618
body: FileManageOperationInput,
572619
context: FileManageOperationContext
@@ -638,14 +685,24 @@ export async function executeFileManageOperation(
638685
}
639686

640687
case 'read': {
641-
const { fileId, fileInput } = body
688+
const { fileId, fileInput, folderPaths, includeSubfolders } = body
642689
const selectedFileIds = Array.isArray(fileId)
643690
? fileId.map((id) => id.trim()).filter(Boolean)
644691
: fileId
645692
? normalizeFileIdList(fileId)
646693
: extractFileIdsFromInput(fileInput)
647694
const selectedInputFiles = fileId ? [] : extractUserFilesFromInput(fileInput)
648695

696+
signal?.throwIfAborted()
697+
for (const id of await expandFolderPathsToFileIds({
698+
principal,
699+
workspaceId,
700+
folderPaths,
701+
includeSubfolders,
702+
})) {
703+
if (!selectedFileIds.includes(id)) selectedFileIds.push(id)
704+
}
705+
649706
if (selectedFileIds.length === 0 && selectedInputFiles.length === 0) {
650707
return Response.json({ success: false, error: 'File is required' }, { status: 400 })
651708
}
@@ -729,14 +786,24 @@ export async function executeFileManageOperation(
729786
}
730787

731788
case 'content': {
732-
const { fileId, fileInput } = body
789+
const { fileId, fileInput, folderPaths, includeSubfolders } = body
733790
const selectedFileIds = Array.isArray(fileId)
734791
? fileId.map((id) => id.trim()).filter(Boolean)
735792
: fileId
736793
? normalizeFileIdList(fileId)
737794
: extractFileIdsFromInput(fileInput)
738795
const selectedInputFiles = fileId ? [] : extractUserFilesFromInput(fileInput)
739796

797+
signal?.throwIfAborted()
798+
for (const id of await expandFolderPathsToFileIds({
799+
principal,
800+
workspaceId,
801+
folderPaths,
802+
includeSubfolders,
803+
})) {
804+
if (!selectedFileIds.includes(id)) selectedFileIds.push(id)
805+
}
806+
740807
if (selectedFileIds.length === 0 && selectedInputFiles.length === 0) {
741808
return contentResponse({ success: false, error: 'File is required' }, { status: 400 })
742809
}
@@ -828,7 +895,7 @@ export async function executeFileManageOperation(
828895
}
829896

830897
case 'write': {
831-
const { fileName, content, fileInput, contentType, overwrite } = body
898+
const { fileName, content, fileInput, contentType, overwrite, folderPath } = body
832899
signal?.throwIfAborted()
833900
const provenanceResolution = resolveFileWriteSecretProvenance({
834901
headers,
@@ -920,7 +987,16 @@ export async function executeFileManageOperation(
920987
? mergeWorkspaceFileSecretProvenance(...writeProvenanceSources)
921988
: undefined
922989

923-
const { folderSegments, leafName } = splitWorkspaceFilePath(sourceName ?? '')
990+
const { folderSegments: nameSegments, leafName } = splitWorkspaceFilePath(sourceName ?? '')
991+
/*
992+
* The destination is the picked folder, then whatever folders the name
993+
* itself spells. `folderPath` is canonical and percent-encoded, so it is
994+
* decoded to names here — the same names `splitWorkspaceFilePath` yields
995+
* — because the folder operation takes decoded segments.
996+
*/
997+
const folderSegments = folderPath
998+
? [...parseFolderPath(folderPath), ...nameSegments]
999+
: nameSegments
9241000
await admitCreateWorkspaceFile(principal, workspaceId)
9251001
const { folderId } = await ensureWorkspaceFileFolderPathOperation.execute({
9261002
principal,
@@ -1206,14 +1282,24 @@ export async function executeFileManageOperation(
12061282
}
12071283

12081284
case 'compress': {
1209-
const { fileId, fileInput, archiveName } = body
1285+
const { fileId, fileInput, archiveName, folderPaths, includeSubfolders } = body
12101286
const selectedFileIds = Array.isArray(fileId)
12111287
? fileId.map((id) => id.trim()).filter(Boolean)
12121288
: fileId
12131289
? normalizeFileIdList(fileId)
12141290
: extractFileIdsFromInput(fileInput)
12151291
const selectedInputFiles = fileId ? [] : extractUserFilesFromInput(fileInput)
12161292

1293+
signal?.throwIfAborted()
1294+
for (const id of await expandFolderPathsToFileIds({
1295+
principal,
1296+
workspaceId,
1297+
folderPaths,
1298+
includeSubfolders,
1299+
})) {
1300+
if (!selectedFileIds.includes(id)) selectedFileIds.push(id)
1301+
}
1302+
12171303
if (selectedFileIds.length === 0 && selectedInputFiles.length === 0) {
12181304
return Response.json({ success: false, error: 'File is required' }, { status: 400 })
12191305
}

‎apps/sim/lib/internal/tool-operations/registry.server.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -837,6 +837,12 @@ const FILE_TOOL_IDS = [
837837
'file_parser',
838838
'file_parser_v2',
839839
'file_parser_v3',
840+
'file_list',
841+
'file_create_folder',
842+
'file_update_folder',
843+
'file_delete_folder',
844+
'file_restore_folder',
845+
'file_move',
840846
] as const
841847

842848
const UPTIMEROBOT_TOOL_IDS = ['uptimerobot_create_psp', 'uptimerobot_update_psp'] as const

0 commit comments

Comments
 (0)