Repository navigation
Description
The FILE instrument schema validates each file group in isolation, so the API stores instruments that no one can complete. count.min and count.max are checked as two independent positive integers, and nothing requires min <= max. A group declared { min: 3, max: 1 } passes $AnyInstrument. The Dropzone then caps selection at max (1 file) while submit requires at least min (3), so every attempt fails with "You uploaded 1 file, but between 3 and 1 are required". Basenames are not checked for uniqueness across groups either. Two groups with the same basename share one uploadMap[basename] entry in the renderer, so choosing files in one dropzone replaces the other's. The API presigns both groups onto the same storage keys, and validateFiles pushes the same uploads once per group, so the record gets duplicate file rows. Both mistakes are accepted at upload, with no error for the author. They only surface when a subject tries to complete the instrument.
Where
packages/schemas/src/instrument/instrument.file.ts:12-22:
const $$FileGroup = <TLanguage extends InstrumentLanguage>(language?: TLanguage) => {
return z.object({
basename: z.string(),
count: z.object({
max: z.int().positive(),
min: z.int().positive()
}),
label: $$InstrumentUIOption(z.string(), language),
type: $FileType.nullable()
}) satisfies z.ZodType<FileInstrument.FileGroup>;
};content: z.object({ fileGroups: z.array($$FileGroup(language)) }) at :26-28 has no cross-group check. The consumers that assume both rules hold:
packages/react-core/src/components/FileInstrumentContent/Dropzone.tsx:51and:61key state bybasename.apps/api/src/instrument-records/files/files.service.ts:87keys presigned URLs bybasename, and:174-185validates and collects files perbasename.
Reproduce
- Upload a FILE instrument whose
fileGroupsis[{ basename: 'scan', count: { min: 3, max: 1 }, label: 'Scan', type: null }]. Upload a second one whose two groups are bothbasename: 'scan', each with{ min: 1, max: 1 }. - Start a session and open each instrument.
Actual: both instruments are accepted. The first can never be submitted. In the second, selecting a file in one dropzone shows it in both, and a completed upload stores the same files twice.
Expected: POST /v1/instruments rejects both bundles with a validation issue that points at the offending group.
Tests
packages/schemas/src/instrument/__tests__/instrument.file.test.ts (new, beside the other instrument.*.test.ts files):
it('should reject a file group whose minimum count exceeds its maximum, since no upload could satisfy it')it('should reject two file groups with the same basename, since their uploads would overwrite each other')
E2E in testing/: with the admin token, POST /v1/instruments a bundle whose file groups share a basename, and assert a 422 (the path the playground's Upload Bundle dialog uses).
Suggested fix
Add a .refine to count for min <= max, and a .check on fileGroups that reports duplicate basenames by index. Both survive z.toJSONSchema, which apps/outreach requires. While here, decide whether min: 0 (an optional group) is legal. The schema's positive() rejects it, but FilesService.validateFiles, the Dropzone's "Between 0 and N" label and #1648 all treat it as valid.
Activity
joshunrau commentedon Oct 26, 2026
Still reproduces on
main(d86740e). No commit since this was filed touchespackages/schemas/src/instrument/,FileInstrumentContent/,instrument-records/files/orstorage/, and no open PR changes the schema.min: 0I'd make it legal. Both bounds have been
positive()since the FILE kind was added (21695b3), and every path that stores an instrument (the controller, repo sync, demo seeding) goes throughInstrumentsService.create. So no stored group hasmin: 0, and relaxing it needs no migration. The zero-file case already works in the count checks atFileInstrumentContent/store.ts:26andfiles.service.ts:177, and in the "No files" state atapps/web/src/routes/_app/datahub/subjects/$subjectId/table/$recordId.tsx:84-87. The one exception is the crash in #1648.Use
min: z.int().nonnegative()and keepmaxpositive(). react-dropzone 20.1.2 treatsmaxFiles: 0as no limit, somax: 0would remove the Dropzone's cap. Fix #1648 in the same PR. If groups should stay mandatory instead, close #1648 as unreachable and drop themin: 0fixtures infiles.service.test.ts(:146, :210, :310, :325).Notes on the fix
countwithpath: ['min'], and have thefileGroupscheck report each duplicate at[index, 'basename']. Through the discriminated union, these come back ascontent.fileGroups.<i>.count.minandcontent.fileGroups.<i>.basename(checked against the installed zod 3.25.76).z.toJSONSchemadoes not throw on either rule, but it leaves both out of its output. The JSON Schema published byapps/outreach/src/pages/resources/schemas/[kind]-instrument-[lang].json.ts:25will keep accepting these groups. JSON Schema cannot express either rule.optionstoInstrumentRenderer(apps/web/src/routes/_app/instruments/render/$id.tsx:115-121), so a broken instrument already on an instance keeps rendering after the fix. Re-uploading the same edition returns 409, so the author has to bumpinternal.edition. The playground preview does validate (apps/playground/src/preview/PreviewApp.tsx:67).FilesService.find(files.service.ts:55-59) gives each group every row with that basename, so the page shows two sections and lists each file twice in each one.fileGroupsaccepts[], though the guidelines say "one or more" (packages/instrument-guidelines/AGENTS.md:575).maxhas no upper bound, andgetPresignedUploadUrlssignscount.maxURLs on every request (files.service.ts:88). Whatever rules land, write them into thecountcomment at:584. That package is published, so the change ships with a release.Tests
instrument.file.test.tsonbilingualFileInstrumentfrom@opendatacapture/instrument-stubs/file, asinstrument.core.test.tsdoes. Add a reject case formax: 0, and an accept case formin: 0if it becomes legal.testing/src/specs/admin-instruments.spec.ts, withadminTokenandapiRequestContextas the archive test uses them (:63-67), sotesting/src/support/api-client.ts(changed by feat: add the instrument hub and record provenance #1807) stays untouched. Fetch the seededARBITRARY_SINGLE_FILEbundle and post it back twice: once withcount:{max:1,min:1}rewritten tomin:3, and once with its group duplicated. Onmainboth return 409, because validation passes andcreatereaches the duplicate-id check (instruments.service.ts:111-113).authorization.spec.ts:399-407already uses this re-post pattern. After the fix both return 422 and nothing is stored, so the test needs no unique name. Assert the issue paths as well as the status.#1712 changes how
$AnyInstrumentis built. If it lands as the union alternative, zod reports these issues inside a singleinvalid_unionat path[], so whichever PR lands second has to update the path assertions.Triage note written with Claude Code. Code references point at
main@d86740e.