Skip to content

Commit c735ded

Browse files
authored
improvement(cli): accept the file references, list shapes and multi-file values models send (#9589)
* improvement(cli): accept the file references, list shapes and multi-file values models send files batch-delete answers to --ids and --paths, reads a JSON array or a comma-joined run of ids as its entries, and the v2 bulk delete resolves files/ VFS paths through the canonical resolver after authorization, refusing the whole batch when one path names no single live file. tables rows batch-delete answers to --row-ids and the log reads to --workflow-ids, the request fields behind their shortened flags. Workflow edits store a multi-file upload field as the list of file objects the editor writes, and an empty list clears it. A document page selection that lists separate pages says to read one range per call. * refactor(cli): share the path predicate and other-spellings list * test(files): drop unit cases the bulk-delete integration suite proves * test(cli): use a flag batch-delete still refuses as the guessed-flag example * fix(cli): refuse an empty entry in a joined file-reference list * improvement(cli): name the metadata command when an embedded download refuses binary output
1 parent e53bf39 commit c735ded

26 files changed

Lines changed: 560 additions & 78 deletions

‎apps/docs/content/docs/cli/files.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,7 @@ sim files batch-delete [file-ids...] [options]
643643

644644
| Argument | Required | Description |
645645
| --- | --- | --- |
646-
| `file-ids...` | No | File identifiers to delete. Same as --file-ids. |
646+
| `file-ids...` | No | Files to delete, each by identifier or by the VFS path of a live file: `files/<folder>/<name>`. Pass the id when two files in a folder have names that display alike. At most 100 entries may be paths. Same as --file-ids. |
647647

648648
</CommandTable>
649649

@@ -653,7 +653,7 @@ sim files batch-delete [file-ids...] [options]
653653

654654
| Option | Required | Description |
655655
| --- | --- | --- |
656-
| `--file-ids <value...>` | No | File identifiers to delete. (space-separated, or @path / @- with one value per line; @@value for a literal leading @) (required, or pass the values as arguments). |
656+
| `--file-ids <value...>` | No | Files to delete, each by identifier or by the VFS path of a live file: `files/<folder>/<name>`. Pass the id when two files in a folder have names that display alike. At most 100 entries may be paths. (space-separated, or @path / @- with one value per line; @@value for a literal leading @) (required, or pass the values as arguments). |
657657
| `-y, --yes` | Yes | Confirm this operation. |
658658

659659
</CommandTable>

‎apps/docs/content/docs/cli/reference.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -981,7 +981,7 @@ sim files batch-delete [file-ids...] [options]
981981

982982
| Argument | Required | Description |
983983
| --- | --- | --- |
984-
| `file-ids...` | No | File identifiers to delete. Same as --file-ids. |
984+
| `file-ids...` | No | Files to delete, each by identifier or by the VFS path of a live file: `files/<folder>/<name>`. Pass the id when two files in a folder have names that display alike. At most 100 entries may be paths. Same as --file-ids. |
985985

986986
</CommandTable>
987987

@@ -991,7 +991,7 @@ sim files batch-delete [file-ids...] [options]
991991

992992
| Option | Required | Description |
993993
| --- | --- | --- |
994-
| `--file-ids <value...>` | No | File identifiers to delete. (space-separated, or @path / @- with one value per line; @@value for a literal leading @) (required, or pass the values as arguments). |
994+
| `--file-ids <value...>` | No | Files to delete, each by identifier or by the VFS path of a live file: `files/<folder>/<name>`. Pass the id when two files in a folder have names that display alike. At most 100 entries may be paths. (space-separated, or @path / @- with one value per line; @@value for a literal leading @) (required, or pass the values as arguments). |
995995
| `-y, --yes` | Yes | Confirm this operation. |
996996

997997
</CommandTable>

‎apps/docs/openapi-v2-files-audit.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7369,7 +7369,7 @@
73697369
"type": "string",
73707370
"minLength": 1
73717371
},
7372-
"description": "File identifiers to delete."
7372+
"description": "Files to delete, each by identifier or by the VFS path of a live file: `files/<folder>/<name>`. Pass the id when two files in a folder have names that display alike. At most 100 entries may be paths."
73737373
}
73747374
},
73757375
"required": ["workspaceId", "fileIds"],

‎apps/sim/app/api/v2/files/bulk-delete/route.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ export const POST = defineV2JsonRoute({
1313
operation: fileOperations.delete,
1414
rateLimit: v2RateLimits.publicApi,
1515
errorPolicy: v2FileErrorPolicies.default,
16-
mapInput: ({ body }) => ({ workspaceId: body.workspaceId, fileIds: body.fileIds }),
16+
mapInput: ({ body }) => ({
17+
workspaceId: body.workspaceId,
18+
fileIds: body.fileIds,
19+
acceptVfsPath: true,
20+
}),
1721
useCase: archiveWorkspaceFileItemsOperation,
1822
present: ({ deletedItems }) => ({ data: { deletedItems: { files: deletedItems.files } } }),
1923
})

‎apps/sim/lib/api/contracts/v2/files.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,10 @@ import { MAX_FOLDER_PATH_BYTES, MAX_FOLDER_PATH_SEGMENTS } from '@/lib/folders/p
4646
import { MAX_WORKSPACE_FILE_SIZE } from '@/lib/uploads/shared/types'
4747
import { MAX_TEXT_EXTRACTION_BYTES } from '@/lib/uploads/utils/file'
4848
import { isWorkspaceFileVfsPath } from '@/lib/vfs/path'
49-
import { MAX_ZIP_DOWNLOAD_FILES } from '@/lib/workspace-files/limits'
49+
import {
50+
MAX_WORKSPACE_FILE_BULK_REQUEST_PATHS,
51+
MAX_ZIP_DOWNLOAD_FILES,
52+
} from '@/lib/workspace-files/limits'
5053
import {
5154
FILE_SEARCH_DEFAULT_MAX_RESULTS,
5255
FILE_SEARCH_MAX_QUERY_LENGTH,
@@ -527,7 +530,9 @@ export type V2MoveFileItemsResult = z.output<typeof v2MoveFileItemsResultSchema>
527530
export const v2BulkDeleteFilesBodySchema = z
528531
.object({
529532
workspaceId: workspaceIdSchema.describe('Workspace containing the files.'),
530-
fileIds: fileIdsSchema.describe('File identifiers to delete.'),
533+
fileIds: fileIdsSchema.describe(
534+
`Files to delete, each by identifier or by the VFS path of a live file: \`files/<folder>/<name>\`. Pass the id when two files in a folder have names that display alike. At most ${MAX_WORKSPACE_FILE_BULK_REQUEST_PATHS} entries may be paths.`
535+
),
531536
})
532537
.strict()
533538

‎apps/sim/lib/workflows/editing/validation.test.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,43 @@ describe('validateInputsForBlock', () => {
345345
).toBe(false)
346346
})
347347

348+
describe('file-upload values', () => {
349+
const multiple = { id: 'attachmentFiles', type: 'file-upload' as const, multiple: true }
350+
const single = { id: 'file', type: 'file-upload' as const }
351+
const stored = {
352+
id: 'wf_a',
353+
name: 'brief.txt',
354+
path: '/api/files/serve/workspace%2Fws%2Fbrief.txt?context=workspace',
355+
key: 'workspace/ws/brief.txt',
356+
size: 12,
357+
type: 'text/plain',
358+
}
359+
const validate = (config: typeof multiple | typeof single, value: unknown) =>
360+
validateValueForSubBlockType(config, value, config.id, 'agent', 'agent-1')
361+
362+
it('stores a multi-file field as the list of file objects the editor writes', () => {
363+
const files = [stored, { ...stored, id: 'wf_b', name: 'notes.txt' }]
364+
expect(validate(multiple, files)).toEqual({ valid: true, value: files })
365+
})
366+
367+
it('clears a multi-file field given an empty list, as removing its last file does', () => {
368+
expect(validate(multiple, [])).toEqual({ valid: true, value: null })
369+
})
370+
371+
it('names the entry of a multi-file list that is not a stored file', () => {
372+
const result = validate(multiple, [stored, { name: 'notes.txt' }])
373+
expect(result.valid).toBe(false)
374+
expect(result.error?.error).toContain('attachmentFiles[1]')
375+
expect(validate(multiple, [stored, 'brief.txt']).valid).toBe(false)
376+
})
377+
378+
it('keeps a single-file field to one file object', () => {
379+
expect(validate(single, stored)).toEqual({ valid: true, value: stored })
380+
expect(validate(single, [stored]).valid).toBe(false)
381+
expect(validate(single, []).valid).toBe(false)
382+
})
383+
})
384+
348385
it('stores input-format rows with legacy or aliased types unchanged', () => {
349386
const rows = [
350387
{ name: 'payload', type: 'json' },

‎apps/sim/lib/workflows/editing/validation.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,11 @@ function validateFallbackModelEntry(item: any, index: number): string | null {
516516
return null
517517
}
518518

519+
/** A file object as the file-upload editor stores it: at least its name and serve path. */
520+
function isStoredFile(value: unknown): boolean {
521+
return isPlainRecord(value) && Boolean(value.name) && Boolean(value.path)
522+
}
523+
519524
/**
520525
* Validates a value against its expected subBlock type
521526
* Returns validation result with the value or an error
@@ -609,30 +614,31 @@ export function validateValueForSubBlockType(
609614
}
610615

611616
case 'file-upload': {
612-
// File upload should be an object with specific properties or null
613-
if (value === null) return { valid: true, value: null }
614-
if (typeof value !== 'object') {
617+
// Removing the editor's last file stores null, so an empty list clears the field too.
618+
if (subBlockConfig.multiple && Array.isArray(value)) {
619+
if (value.length === 0) return { valid: true, value: null }
620+
const index = value.findIndex((file) => !isStoredFile(file))
621+
if (index === -1) return { valid: true, value }
615622
return {
616623
valid: false,
617624
error: {
618625
blockId,
619626
blockType,
620627
field: fieldName,
621628
value,
622-
error: `Invalid file-upload value for field "${fieldName}" - expected object with name and path properties, or null`,
629+
error: `Invalid file-upload entry ${fieldName}[${index}] - each file must be an object with "name" and "path" properties`,
623630
},
624631
}
625632
}
626-
// Validate file object has required properties
627-
if (value && (!value.name || !value.path)) {
633+
if (!isStoredFile(value)) {
628634
return {
629635
valid: false,
630636
error: {
631637
blockId,
632638
blockType,
633639
field: fieldName,
634640
value,
635-
error: `Invalid file-upload object for field "${fieldName}" - must have "name" and "path" properties`,
641+
error: `Invalid file-upload value for field "${fieldName}" - expected ${subBlockConfig.multiple ? 'a list of objects' : 'one object'} with "name" and "path" properties, or null`,
636642
},
637643
}
638644
}

‎apps/sim/lib/workspace-files/__integration__/file-path-references.integration.ts‎

Lines changed: 92 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
/**
2-
* Real PostgreSQL behavior of single-file operations addressed by a `files/…` VFS path instead
3-
* of an id: the path names exactly one live file in the asserted workspace, never a same-named
4-
* file in another folder or workspace, a chat upload or an archived file, and authorization is
5-
* unchanged. A caller who cannot reach the workspace learns nothing about the paths in it.
2+
* Real PostgreSQL behavior of file operations addressed by a `files/…` VFS path instead of an id:
3+
* the path names exactly one live file in the asserted workspace, never a same-named file in
4+
* another folder or workspace, a chat upload or an archived file, and authorization is
5+
* unchanged. A caller who cannot reach the workspace learns nothing about the paths in it, and a
6+
* bulk delete with one unresolved path deletes nothing.
67
*/
78
import { db } from '@sim/db'
89
import { permissions, user, workspace, workspaceFiles } from '@sim/db/schema'
@@ -13,6 +14,7 @@ import { eq, inArray, sql } from 'drizzle-orm'
1314
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
1415
import { MAX_FOLDERS_PER_WORKSPACE } from '@/lib/folders/constants'
1516
import { workspaceFileVfsPath } from '@/lib/uploads/contexts/workspace/file-manager'
17+
import { archiveWorkspaceFileItemsOperation } from '@/lib/workspace-files/application/archive-workspace-file-items'
1618
import { deleteWorkspaceFileOperation } from '@/lib/workspace-files/application/delete-workspace-file'
1719
import { readWorkspaceFileMetadataWithVersion } from '@/lib/workspace-files/application/read-workspace-file-metadata'
1820
import { renameWorkspaceFile } from '@/lib/workspace-files/application/rename-workspace-file'
@@ -64,6 +66,12 @@ const deleteFile = (fileId: string, userId = alice) =>
6466
input: { fileId, acceptVfsPath: true, assertedWorkspaceId: workspaceId },
6567
})
6668

69+
const bulkDelete = (fileIds: string[], userId = alice, acceptVfsPath = true) =>
70+
archiveWorkspaceFileItemsOperation.execute({
71+
principal: session(userId),
72+
input: { workspaceId, fileIds, acceptVfsPath },
73+
})
74+
6775
/** The `{ code, message }` each path is refused with, or null where it resolved. */
6876
const refusalsFor = (
6977
principal: Parameters<typeof readWorkspaceFileMetadataWithVersion.execute>[0]['principal'],
@@ -373,4 +381,84 @@ describe('single-file operations addressed by VFS path', () => {
373381
})
374382
expect((await row(ids.rootReport))?.deletedAt).toBeNull()
375383
})
384+
385+
describe('bulk delete', () => {
386+
const bulk = { folder: generateId(), one: '', two: '', three: '', rootOne: '' }
387+
388+
beforeAll(async () => {
389+
await db.execute(sql`
390+
INSERT INTO folder (id, name, user_id, workspace_id, resource_type, parent_id)
391+
VALUES (${bulk.folder}, 'Bulk', ${alice}, ${workspaceId}, 'file', NULL)`)
392+
bulk.one = await insertFile({ name: 'one.csv', folderId: bulk.folder })
393+
bulk.two = await insertFile({ name: 'two.csv', folderId: bulk.folder })
394+
bulk.three = await insertFile({ name: 'three.csv', folderId: bulk.folder })
395+
bulk.rootOne = await insertFile({ name: 'one.csv' })
396+
})
397+
398+
const live = async (ids: string[]) =>
399+
Promise.all(ids.map(async (id) => (await row(id))?.deletedAt === null))
400+
401+
it('refuses the whole request when one path names no single live file', async () => {
402+
await expect(
403+
bulkDelete(['files/Bulk/three.csv', 'files/Bulk/missing.csv'])
404+
).rejects.toMatchObject({
405+
code: 'not_found',
406+
message: 'No live file at "files/Bulk/missing.csv"',
407+
})
408+
await expect(
409+
bulkDelete([bulk.three, 'files/Bulk/one.csv,files/Bulk/two.csv'])
410+
).rejects.toMatchObject({
411+
message: expect.stringContaining('pass each path as its own entry'),
412+
})
413+
await expect(bulkDelete([bulk.three, await ambiguousPath()])).rejects.toMatchObject({
414+
code: 'validation',
415+
})
416+
await expect(bulkDelete([bulk.three, 'files/notes.txt'])).rejects.toMatchObject({
417+
code: 'not_found',
418+
})
419+
expect(await live([bulk.one, bulk.two, bulk.three, ids.chatUpload])).toEqual([
420+
true,
421+
true,
422+
true,
423+
true,
424+
])
425+
})
426+
427+
it('keeps authorization and answers a non-member the same for every path', async () => {
428+
await expect(bulkDelete(['files/Bulk/one.csv'], bob)).rejects.toMatchObject({
429+
code: 'forbidden',
430+
})
431+
const refusal = (fileIds: string[]) =>
432+
bulkDelete(fileIds, carol).then(
433+
() => null,
434+
(error: { code?: string; message?: string }) => ({
435+
code: error.code,
436+
message: error.message,
437+
})
438+
)
439+
const existing = await refusal(['files/Bulk/one.csv', 'files/Bulk/two.csv'])
440+
expect(existing).toMatchObject({ code: 'forbidden' })
441+
expect(await refusal(['files/Bulk/one.csv', 'files/Bulk/missing.csv'])).toEqual(existing)
442+
expect(await live([bulk.one, bulk.two])).toEqual([true, true])
443+
})
444+
445+
it('resolves no path for a caller that did not opt in', async () => {
446+
await expect(bulkDelete(['files/Bulk/one.csv'], alice, false)).rejects.toMatchObject({
447+
code: 'not_found',
448+
})
449+
expect(await live([bulk.one])).toEqual([true])
450+
})
451+
452+
it('deletes the files a mix of paths and ids names, and no same-named file elsewhere', async () => {
453+
expect(
454+
await bulkDelete(['files/Bulk/one.csv', bulk.two, 'files/Bulk/two.csv'])
455+
).toMatchObject({ deletedItems: { files: 2 } })
456+
expect(await live([bulk.one, bulk.two, bulk.three, bulk.rootOne])).toEqual([
457+
false,
458+
false,
459+
true,
460+
true,
461+
])
462+
})
463+
})
376464
})

‎apps/sim/lib/workspace-files/application/archive-workspace-file-items.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ import {
55
import { auditMock, auditMockFns } from '@sim/testing/mocks/audit.mock'
66
import { realtimeNotifyMock, realtimeNotifyMockFns } from '@sim/testing/mocks/realtime-notify.mock'
77
import { workspaceAuthzMock, workspaceAuthzMockFns } from '@sim/testing/mocks/workspace-authz.mock'
8+
import {
9+
workspaceFileManagerMock,
10+
workspaceFileManagerMockFns,
11+
} from '@sim/testing/mocks/workspace-file-manager.mock'
812
import {
913
workspaceUploadsMock,
1014
workspaceUploadsMockFns,
@@ -16,6 +20,7 @@ const { events } = vi.hoisted(() => ({
1620
}))
1721

1822
vi.mock('@/lib/uploads/contexts/workspace', () => workspaceUploadsMock)
23+
vi.mock('@/lib/uploads/contexts/workspace/file-manager', () => workspaceFileManagerMock)
1924
vi.mock('@sim/platform-authz/workspace', () => workspaceAuthzMock)
2025
vi.mock('@sim/audit', () => auditMock)
2126
vi.mock('@/lib/realtime/notify', () => realtimeNotifyMock)
@@ -31,6 +36,7 @@ const mockResolvePermission = workspaceAuthzMockFns.mockResolveEffectiveWorkspac
3136
const mockAssertItems = workspaceUploadsMockFns.mockAssertWorkspaceFileItemsBelongToWorkspace
3237
const mockArchive = workspaceUploadsMockFns.mockBulkArchiveWorkspaceFileItems
3338
const mockLoadContext = workspaceUploadsMockFns.mockLoadWorkspaceFileOperationContext
39+
const mockResolveReference = workspaceFileManagerMockFns.mockResolveFileReference
3440

3541
describe('archiveWorkspaceFileItemsOperation', () => {
3642
beforeEach(() => {
@@ -57,6 +63,10 @@ describe('archiveWorkspaceFileItemsOperation', () => {
5763
fileIds: ['file-1'],
5864
folderIds: [],
5965
}))
66+
mockResolveReference.mockImplementation(async (_owner: unknown, reference: string) => {
67+
events.push(`lookup ${reference}`)
68+
return reference === 'files/Docs/a.csv' ? { id: 'file-a' } : null
69+
})
6070
})
6171

6272
it('authorizes a delegated bulk selection without borrowing the first file scope', async () => {
@@ -151,4 +161,30 @@ describe('archiveWorkspaceFileItemsOperation', () => {
151161
expect(events).toEqual(['resolve', 'authorize'])
152162
expect(mockArchive).not.toHaveBeenCalled()
153163
})
164+
165+
describe('with files/ paths', () => {
166+
const deleteByReference = (fileIds: string[], acceptVfsPath = true) =>
167+
archiveWorkspaceFileItemsOperation.execute({
168+
principal: createSessionPrincipal(),
169+
input: { workspaceId: 'ws-1', fileIds, acceptVfsPath },
170+
})
171+
172+
it('looks up no path for a caller it refuses', async () => {
173+
mockResolvePermission.mockImplementation(async () => {
174+
events.push('authorize')
175+
return 'read'
176+
})
177+
await expect(deleteByReference(['files/Docs/a.csv'])).rejects.toMatchObject({
178+
code: 'forbidden',
179+
})
180+
expect(events).toEqual(['resolve', 'authorize'])
181+
})
182+
183+
it('refuses more paths than one request resolves before looking any up', async () => {
184+
await expect(
185+
deleteByReference(Array.from({ length: 101 }, (_, index) => `files/f-${index}.csv`))
186+
).rejects.toThrow('at most 100 file paths')
187+
expect(mockResolveReference).not.toHaveBeenCalled()
188+
})
189+
})
154190
})

0 commit comments

Comments
 (0)