mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 08:01:35 +00:00
fix: keep a colliding oneOf key against the branch whose shape it fits
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
d71cc28e5c
commit
fd57dc00be
@@ -106,9 +106,9 @@
|
||||
<div class="mt-3">
|
||||
{#if hasArgs}
|
||||
<!-- The one thing here that runs before Run: a `dynselect-`/`dynmultiselect-`
|
||||
argument makes DynamicInput execute that entrypoint on mount to fill its options,
|
||||
so opening this card runs it, and Cancel does not undo it. Everything else waits
|
||||
for the user, and a new surface added here must keep it that way. -->
|
||||
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. -->
|
||||
<SchemaForm
|
||||
bind:schema
|
||||
helperScript={{ source: 'deployed', path: runForm.path, runnable_kind: 'script' }}
|
||||
|
||||
@@ -71,6 +71,50 @@ describe('conformArgsToSchema', () => {
|
||||
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', () => {
|
||||
|
||||
@@ -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<string, any> = {}
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user