diff --git a/frontend/src/lib/components/copilot/chat/RunArgsFormDisplay.svelte b/frontend/src/lib/components/copilot/chat/RunArgsFormDisplay.svelte index d19837907c..8b4500c879 100644 --- a/frontend/src/lib/components/copilot/chat/RunArgsFormDisplay.svelte +++ b/frontend/src/lib/components/copilot/chat/RunArgsFormDisplay.svelte @@ -106,9 +106,9 @@
{#if hasArgs} + argument makes DynamicInput execute that entrypoint on mount to fill its options — + a real job on the deployed script, carrying the other args as proposed, and Cancel + does not undo it. Everything else waits for the user; keep it that way. --> { expect(dropped.undeclared).toEqual(['either.evil']) }) + // Merging the branches last-writer-wins validated the value against a branch the user + // never opened, so submitting deleted what they had just filled in — and blamed the + // script for it. Both orders, because either branch can be the one that loses. + it('shape-checks a colliding oneOf key against the branch it fits', () => { + const obj = { + title: 'obj', + properties: { src: { properties: { bucket: { type: 'string' } } } } + } + const list = { title: 'list', properties: { src: { items: { type: 'string' } } } } + for (const branches of [ + [obj, list], + [list, obj] + ]) { + const schema = { properties: { either: { oneOf: branches } } } + expect( + conformArgsToSchema({ either: { kind: 'obj', src: { bucket: 'b' } } }, schema) + ).toEqual({ + args: { either: { kind: 'obj', src: { bucket: 'b' } } }, + resetKeys: [], + dropped: { undeclared: [], unshowable: [] } + }) + expect(conformArgsToSchema({ either: { kind: 'list', src: ['a'] } }, schema).args).toEqual({ + either: { kind: 'list', src: ['a'] } + }) + } + // Fitting no branch is still unshowable: the collision widens what the form can + // show, it does not stop dropping what it cannot. + expect( + conformArgsToSchema( + { either: { kind: 'a', src: { evil: 1 } } }, + { + properties: { + either: { + oneOf: [ + { title: 'a', properties: { src: { type: 'string' } } }, + { title: 'b', properties: { src: { type: 'number' } } } + ] + } + } + } + ) + ).toMatchObject({ args: { either: { kind: 'a' } }, dropped: { unshowable: ['either.src'] } }) + }) + // A value shaped unlike its schema matches no level below, so every filter walked // past it and the form rendered nothing over an argument the run still carried. it('drops a value whose shape contradicts the declared one', () => { diff --git a/frontend/src/lib/components/job_args.ts b/frontend/src/lib/components/job_args.ts index 69fbaf345f..5b4b6c3702 100644 --- a/frontend/src/lib/components/job_args.ts +++ b/frontend/src/lib/components/job_args.ts @@ -109,20 +109,30 @@ const SCALAR_TYPES = new Set(['string', 'number', 'integer', 'boolean']) const declaresDynMultiselect = (prop: any) => typeof prop?.format === 'string' && prop.format.startsWith('dynmultiselect-') -function dropUndeclaredNested(value: any, prop: any, dropped: DroppedPaths, path: string): any { - if (value == null) return value - if (declaresDynMultiselect(prop) && !Array.isArray(value)) return DROP - if (typeof value !== 'object') return value - // A value the form cannot show is one the user would approve unseen: it matches no - // level below, so every filter falls straight through it, and `ArgInput` binds it to - // a widget that renders nothing — an object in a list slot, or in a scalar input. - if (SCALAR_TYPES.has(prop?.type)) return DROP +/** + * Whether `prop` declares a slot the form can show `value` in. A value that fits nowhere + * is one the user would approve unseen: it matches no level below, so every filter falls + * straight through it, and `ArgInput` binds it to a widget that renders nothing — an + * object in a list slot, or in a scalar input. + */ +function fitsDeclaredShape(value: any, prop: any): boolean { + if (value == null) return true + if (declaresDynMultiselect(prop)) return Array.isArray(value) + if (typeof value !== 'object') return true + if (SCALAR_TYPES.has(prop?.type)) return false const isArray = Array.isArray(value) // Declared nested structure, never the declared `type`: a dyn-multiselect argument is // `type: 'object'` holding an array, so reading `type` would drop what the user picked. const declaresArray = prop?.items != null const declaresObject = prop?.properties != null || Array.isArray(prop?.oneOf) - if (isArray ? declaresObject && !declaresArray : declaresArray && !declaresObject) return DROP + return !(isArray ? declaresObject && !declaresArray : declaresArray && !declaresObject) +} + +function dropUndeclaredNested(value: any, prop: any, dropped: DroppedPaths, path: string): any { + if (value == null) return value + if (!fitsDeclaredShape(value, prop)) return DROP + if (typeof value !== 'object') return value + const isArray = Array.isArray(value) if (prop?.properties && !isArray) { return dropUndeclaredArgs(value, prop.properties, dropped, path) } @@ -142,7 +152,19 @@ function dropUndeclaredNested(value: any, prop: any, dropped: DroppedPaths, path // The union of every branch, not the one the tag names: which branch is selected is // runtime state, and pruning by a stale tag would delete what the user typed. const union: Record = {} - for (const branch of prop.oneOf) Object.assign(union, branch?.properties ?? {}) + for (const branch of prop.oneOf) { + for (const [key, declared] of Object.entries(branch?.properties ?? {})) { + // Two branches can declare one key with shapes that exclude each other, so the + // declaration the value fits wins the union: a last-writer-wins merge validates + // what the user filled in against a branch they never opened, and drops it. + const held = union[key] + if ( + held === undefined || + (!fitsDeclaredShape(value[key], held) && fitsDeclaredShape(value[key], declared)) + ) + union[key] = declared + } + } return dropUndeclaredArgs(value, union, dropped, path, ONE_OF_TAG_KEYS) } return value