Skip to content

Commit c17c003

Browse files
authored
feat(input-format): upload files in file fields via the file uploader (#5297)
* feat(input-format): upload files in file fields via the file uploader The file field in the start-block input format only offered a raw JSON editor expecting hand-written base64 objects, which users routinely filled with junk (local paths, raw text, leftover placeholders) and which never actually fed a run. Reuse the existing FileUpload component for file-typed fields: - Add an opt-in controlled mode (value/onValueChange) to FileUpload so it can be embedded where the value lives outside a subblock; store-bound consumers (stt/vision/agent) are unchanged. - Render the uploader for file fields, with a toggle to fall back to the raw JSON editor for power users / legacy values. - Detect file fields by normalized type (file[]/files/file/image) so copilot/API-authored variants render correctly too. - Wire editor-attached files (already uploaded, run-ready) into manual runs via the executor's files channel; chat/API runs still override. Backwards compatible: the inputFormat array shape is unchanged (file values are stored as a JSON string of run-ready file objects); legacy free-form values open in JSON mode so nothing is lost. * refactor(input-format): extract file adapters, share run-input builder, harden types Follow-up cleanup from /simplify + /cleanup review: - Extract pure file adapters into input-format-files.ts (filesToControlValue, controlValueToFiles, serializeInputFormatFiles, defaultFileFieldMode) so they are unit-tested without a DOM; add tests. - Derive InputFormatFile from the canonical executor UserFile so the editor and runtime file shapes can't drift. - Consolidate the two manual run-input builders into one buildInputFormatInput helper so manual run and run-from-block handle files identically. - Narrow file-field detection to the canonical file[] (matches the field-type dropdown and the existing execution/webhook file paths) so no pre-existing non-file[] field changes behavior. - defaultFileFieldMode parses once; the file value is only parsed in upload mode. - Add lib/component unit tests (45 passing). * fix(input-format): require run-ready file shape, don't clobber named files input Addresses review (Greptile P1s): - parseInputFormatFiles now requires the full run-ready shape (id/name/url + finite size + type), so a partial file can't open in uploader mode or reach workflowInput.files only to be rejected by normalizeStartFile (which would silently drop every file). - buildInputFormatInput no longer overwrites a user field literally named 'files' with the upload channel; that reserved-name collision is left for the executor to surface. * fix(input-format): require file key, keep mixed/partial values in JSON mode Addresses review round 2 (Greptile P1 + Cursor): - parseInputFormatFiles now requires a non-empty key (the uploader always sets one); an external/signed URL with no recoverable key no longer opens uploader mode or reaches workflowInput.files only to be rejected. - defaultFileFieldMode uses the uploader only when EVERY entry is run-ready; a mixed array with any legacy/partial entry stays in JSON mode so the uploader can't drop the entries it can't represent on the next save. * fix(input-format): recover internal-url keys, gate uploader to lossless values Addresses review round 3 (Greptile P1 + Cursor): - parseInputFormatFiles accepts a key-less file when its url is an internal /api/files/serve/... url (the executor recovers the key from it, same as normalizeStartFile); only a missing key with a non-internal url is rejected. - The file field offers the uploader only when it can represent the stored value losslessly (empty or all run-ready); for mixed/legacy values it forces JSON mode and hides the toggle, so a sticky 'upload' preference can no longer drop entries the uploader cannot show on save. * fix(input-format): require non-empty id/name/url/type in parseInputFormatFiles Addresses review round 4 (Greptile): normalizeStartFile rejects falsy id/name/url/type and file normalization is all-or-nothing, so an empty-string field (e.g. hand-edited value with an empty id + internal url) would open in uploader mode and drop every file from the run. Require non-empty strings to match the executor. * fix(input-format): validate recovered internal-url keys via parseInternalFileUrl Addresses review round 5 (Greptile): isInternalFileUrl only checks the URL prefix, but the executor parses+decodes the key. A malformed internal URL (no extractable key) passed the prefix check, got collected into workflowInput.files, then normalizeStartFile rejected it and dropped the whole files array. Now mirror the executor exactly — attempt parseInternalFileUrl and require a real key. Tests use a realistic recoverable internal URL. * fix(input-format): uploaded files always own the files channel Addresses review round 6 (Greptile): files is NOT in the executor's reserved input-format names (EXECUTION_CONTROL_OUTPUT_FIELD_NAMES), so the previous guard silently dropped uploaded file[] values whenever a plain field named files coexisted. files is the start block's canonical file channel (the chat trigger names its own file field files), so uploaded files now always populate it and take precedence — dropping real attachments is the worse outcome, and making files a reserved-name error isn't viable since it's the legitimate file-field name. * chore(input-format): tighten parseInputFormatFiles comment
1 parent 32b6a42 commit c17c003

7 files changed

Lines changed: 574 additions & 67 deletions

File tree

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

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,17 @@ interface FileUploadProps {
3939
isPreview?: boolean
4040
previewValue?: any | null
4141
disabled?: boolean
42+
/**
43+
* Controlled value. When `onValueChange` is provided the component reads from
44+
* this prop and writes through `onValueChange` instead of the subblock store,
45+
* letting it be embedded where the value lives outside a subblock (e.g. a
46+
* single field inside the input-format editor).
47+
*/
48+
value?: UploadedFile | UploadedFile[] | null
49+
onValueChange?: (value: UploadedFile | UploadedFile[] | null) => void
4250
}
4351

44-
interface UploadedFile {
52+
export interface UploadedFile {
4553
name: string
4654
path: string
4755
key?: string
@@ -165,9 +173,25 @@ export function FileUpload({
165173
isPreview = false,
166174
previewValue,
167175
disabled = false,
176+
value: controlledValue,
177+
onValueChange,
168178
}: FileUploadProps) {
169179
const activeSearchTarget = useActiveSearchTarget()
170180
const [storeValue, setStoreValue] = useSubBlockValue(blockId, subBlockId)
181+
const isControlled = onValueChange !== undefined
182+
183+
/**
184+
* Persists a new value. In controlled mode the caller owns persistence; in
185+
* store mode we write through the subblock store and notify collaborators.
186+
*/
187+
const commitValue = (next: UploadedFile | UploadedFile[] | null) => {
188+
if (isControlled) {
189+
onValueChange(next)
190+
return
191+
}
192+
setStoreValue(next)
193+
useWorkflowStore.getState().triggerUpdate()
194+
}
171195
const [modelValue] = useSubBlockValue(blockId, 'model')
172196
const [uploadingFiles, setUploadingFiles] = useState<UploadingFile[]>([])
173197
const [uploadProgress, setUploadProgress] = useState(0)
@@ -191,7 +215,7 @@ export function FileUpload({
191215
const uploadFileMutation = useUploadWorkspaceFile()
192216
const queryClient = useQueryClient()
193217

194-
const value = isPreview ? previewValue : storeValue
218+
const value = isControlled ? controlledValue : isPreview ? previewValue : storeValue
195219

196220
const maxSizeInBytes = useMemo(() => {
197221
const fallback = maxSize * 1024 * 1024
@@ -413,11 +437,9 @@ export function FileUpload({
413437

414438
const newFiles = Array.from(uniqueFiles.values())
415439

416-
setStoreValue(newFiles)
417-
useWorkflowStore.getState().triggerUpdate()
440+
commitValue(newFiles)
418441
} else {
419-
setStoreValue(uploadedFiles[0] || null)
420-
useWorkflowStore.getState().triggerUpdate()
442+
commitValue(uploadedFiles[0] || null)
421443
}
422444
} catch (error) {
423445
logger.error(getErrorMessage(error, 'Failed to upload file(s)'), activeWorkflowId)
@@ -459,12 +481,11 @@ export function FileUpload({
459481
uniqueFiles.set(uploadedFile.path, uploadedFile)
460482
const newFiles = Array.from(uniqueFiles.values())
461483

462-
setStoreValue(newFiles)
484+
commitValue(newFiles)
463485
} else {
464-
setStoreValue(uploadedFile)
486+
commitValue(uploadedFile)
465487
}
466488

467-
useWorkflowStore.getState().triggerUpdate()
468489
logger.info(`Selected workspace file: ${selectedFile.name}`, activeWorkflowId)
469490
}
470491

@@ -501,12 +522,10 @@ export function FileUpload({
501522
if (multiple) {
502523
const filesArray = Array.isArray(value) ? value : value ? [value] : []
503524
const updatedFiles = filesArray.filter((f) => f.path !== file.path)
504-
setStoreValue(updatedFiles.length > 0 ? updatedFiles : null)
525+
commitValue(updatedFiles.length > 0 ? updatedFiles : null)
505526
} else {
506-
setStoreValue(null)
527+
commitValue(null)
507528
}
508-
509-
useWorkflowStore.getState().triggerUpdate()
510529
} catch (error) {
511530
logger.error(getErrorMessage(error, 'Failed to remove file'), activeWorkflowId)
512531
} finally {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
import { describe, expect, it } from 'vitest'
2+
import type { InputFormatFile } from '@/lib/workflows/input-format'
3+
import {
4+
controlValueToFiles,
5+
defaultFileFieldMode,
6+
filesToControlValue,
7+
serializeInputFormatFiles,
8+
} from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/starter/input-format-files'
9+
10+
const file: InputFormatFile = {
11+
id: 'f1',
12+
name: 'doc.pdf',
13+
url: '/api/files/serve/workspace%2Fws-1%2F1700000000000-doc.pdf?context=workspace',
14+
key: 'key',
15+
size: 10,
16+
type: 'application/pdf',
17+
}
18+
19+
describe('filesToControlValue', () => {
20+
it.concurrent('maps url -> path for the FileUpload value shape', () => {
21+
expect(filesToControlValue([file])).toEqual([
22+
{
23+
name: file.name,
24+
path: file.url,
25+
key: file.key,
26+
size: file.size,
27+
type: file.type,
28+
},
29+
])
30+
})
31+
32+
it.concurrent('round-trips through controlValueToFiles without data loss', () => {
33+
expect(controlValueToFiles(filesToControlValue([file]), [file])).toEqual([file])
34+
})
35+
})
36+
37+
describe('controlValueToFiles', () => {
38+
it.concurrent('preserves the stable id of an existing file (matched by key)', () => {
39+
const control = [
40+
{ name: 'doc.pdf', path: '/moved', key: 'key', size: 10, type: 'application/pdf' },
41+
]
42+
expect(controlValueToFiles(control, [file])[0].id).toBe('f1')
43+
})
44+
45+
it.concurrent('matches an existing file by url when key is absent', () => {
46+
const control = [{ name: 'doc.pdf', path: file.url, size: 10, type: 'application/pdf' }]
47+
expect(controlValueToFiles(control, [file])[0].id).toBe('f1')
48+
})
49+
50+
it.concurrent('generates an id for a newly added file', () => {
51+
const control = [
52+
{
53+
name: 'new.pdf',
54+
path: '/api/files/serve/new',
55+
key: 'new',
56+
size: 5,
57+
type: 'application/pdf',
58+
},
59+
]
60+
const result = controlValueToFiles(control, [file])
61+
expect(result[0].id).toEqual(expect.any(String))
62+
expect(result[0].id).not.toBe('f1')
63+
expect(result[0].url).toBe('/api/files/serve/new')
64+
})
65+
66+
it.concurrent('normalizes a single object or null to an array', () => {
67+
const single = {
68+
name: file.name,
69+
path: file.url,
70+
key: file.key,
71+
size: file.size,
72+
type: file.type,
73+
}
74+
expect(controlValueToFiles(single, [file])).toEqual([file])
75+
expect(controlValueToFiles(null, [file])).toEqual([])
76+
})
77+
})
78+
79+
describe('serializeInputFormatFiles', () => {
80+
it.concurrent('serializes to JSON that parses back to the same files', () => {
81+
expect(JSON.parse(serializeInputFormatFiles([file]))).toEqual([file])
82+
})
83+
84+
it.concurrent('returns an empty string for no files', () => {
85+
expect(serializeInputFormatFiles([])).toBe('')
86+
})
87+
})
88+
89+
describe('defaultFileFieldMode', () => {
90+
it.concurrent('defaults to upload for empty or whitespace values', () => {
91+
expect(defaultFileFieldMode(undefined)).toBe('upload')
92+
expect(defaultFileFieldMode('')).toBe('upload')
93+
expect(defaultFileFieldMode(' ')).toBe('upload')
94+
})
95+
96+
it.concurrent('uses upload for an empty array or run-ready files', () => {
97+
expect(defaultFileFieldMode('[]')).toBe('upload')
98+
expect(defaultFileFieldMode(JSON.stringify([file]))).toBe('upload')
99+
})
100+
101+
it.concurrent('falls back to json for legacy free-form values (no data loss)', () => {
102+
expect(defaultFileFieldMode('C:/Users/x/budget.xlsx')).toBe('json')
103+
expect(defaultFileFieldMode('[{"data":"<base64>","name":"x.pdf"}]')).toBe('json')
104+
expect(defaultFileFieldMode('{"csv":"a,b,c"}')).toBe('json')
105+
})
106+
107+
it.concurrent('uses json when only some entries are run-ready (no silent drop)', () => {
108+
expect(defaultFileFieldMode(JSON.stringify([file, { name: 'legacy-only' }]))).toBe('json')
109+
})
110+
})
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import { generateId } from '@sim/utils/id'
2+
import { type InputFormatFile, parseInputFormatFiles } from '@/lib/workflows/input-format'
3+
import type { UploadedFile } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/file-upload/file-upload'
4+
5+
/**
6+
* Pure adapters bridging a file-typed input-format field's stored value (a JSON
7+
* string of run-ready {@link InputFormatFile} objects) and the {@link FileUpload}
8+
* component's value shape (which keys off `path`). Kept separate from the React
9+
* component so they can be unit-tested without a DOM.
10+
*/
11+
12+
/**
13+
* Maps stored run-ready file objects to the {@link FileUpload} value shape.
14+
*/
15+
export function filesToControlValue(files: InputFormatFile[]): UploadedFile[] {
16+
return files.map((file) => ({
17+
name: file.name,
18+
path: file.url,
19+
key: file.key,
20+
size: file.size,
21+
type: file.type,
22+
}))
23+
}
24+
25+
/**
26+
* Maps a {@link FileUpload} value back to stored run-ready file objects,
27+
* preserving the stable `id` of files that were already present.
28+
*/
29+
export function controlValueToFiles(
30+
value: UploadedFile | UploadedFile[] | null,
31+
previous: InputFormatFile[]
32+
): InputFormatFile[] {
33+
const uploaded = Array.isArray(value) ? value : value ? [value] : []
34+
return uploaded.map((file) => {
35+
const existing = previous.find(
36+
(prev) => (file.key && prev.key === file.key) || prev.url === file.path
37+
)
38+
return {
39+
id: existing?.id ?? generateId(),
40+
name: file.name,
41+
url: file.path,
42+
key: file.key,
43+
size: file.size,
44+
type: file.type,
45+
}
46+
})
47+
}
48+
49+
/**
50+
* Serializes run-ready file objects into a field value string (empty when none).
51+
*/
52+
export function serializeInputFormatFiles(files: InputFormatFile[]): string {
53+
return files.length > 0 ? JSON.stringify(files, null, 2) : ''
54+
}
55+
56+
/**
57+
* Default editor mode for a file field: the uploader, unless the stored value is
58+
* legacy free-form content (raw text or a non-file array) that only the JSON
59+
* editor can represent without data loss.
60+
*/
61+
export function defaultFileFieldMode(value: string | undefined): 'upload' | 'json' {
62+
if (!value || !value.trim()) return 'upload'
63+
let parsed: unknown
64+
try {
65+
parsed = JSON.parse(value)
66+
} catch {
67+
return 'json'
68+
}
69+
if (!Array.isArray(parsed)) return 'json'
70+
if (parsed.length === 0) return 'upload'
71+
// Only use the uploader when EVERY entry is run-ready; if any entry is legacy
72+
// or partial, stay in JSON mode so the uploader can't drop the entries it
73+
// cannot represent when the field is next saved.
74+
return parseInputFormatFiles(parsed).length === parsed.length ? 'upload' : 'json'
75+
}

0 commit comments

Comments
 (0)