mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-06 08:01:35 +00:00
fix: drop a non-array in a dyn-multiselect slot before it crashes the run form
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
fa587945e8
commit
55156962d6
@@ -3373,7 +3373,7 @@ export class AIChatManager {
|
||||
)
|
||||
if (messages.length === this.messages.length) return
|
||||
checkpointedShape = shape
|
||||
const display = this.settledToolDisplay(this.displayMessages, 'Interrupted')
|
||||
const display = this.#interruptedSnapshot()
|
||||
// onMessageEnd is what gives streamed text its bubble, and it clears
|
||||
// currentReply doing so — text still there has none, and without one the
|
||||
// reply returns as context the reader cannot see.
|
||||
|
||||
@@ -231,28 +231,6 @@ describe('AIChatManager run form', () => {
|
||||
// restored into a fresh manager, which has none of the turn's callbacks. Cancel is
|
||||
// then the card's only exit, and until it settles pendingUserAction keeps the whole
|
||||
// session reading as needs-confirmation.
|
||||
it('settles a restored form whose callback is gone', async () => {
|
||||
const manager = new AIChatManager()
|
||||
manager.displayMessages = [
|
||||
{
|
||||
role: 'tool',
|
||||
tool_call_id: 'call_r',
|
||||
content: 'Waiting for you to confirm the arguments of "f/a/b"',
|
||||
isLoading: true,
|
||||
runForm: { path: 'f/a/b', schema: {}, args: {} }
|
||||
}
|
||||
]
|
||||
expect(manager.isRunFormPending('call_r')).toBe(false)
|
||||
|
||||
manager.handleRunFormCancel('call_r')
|
||||
|
||||
const { pendingUserAction } = await import('./shared')
|
||||
const settled = manager.displayMessages[0]
|
||||
expect(settled.runForm?.canceled).toBe(true)
|
||||
expect(settled.isLoading).toBe(false)
|
||||
expect(pendingUserAction(manager.displayMessages)).toBe(undefined)
|
||||
})
|
||||
|
||||
// A save that fires mid-turn (jobs tray, review dock) stores a transcript nothing
|
||||
// will resume. Storing a card still pending brings back a form whose Run resolves
|
||||
// no callback.
|
||||
@@ -273,9 +251,12 @@ describe('AIChatManager run form', () => {
|
||||
manager.dismissJob('nope')
|
||||
await Promise.resolve()
|
||||
|
||||
const { isActiveRunForm } = await import('./shared')
|
||||
const stored = saveChat.mock.calls.at(-1)?.[0]?.[0]
|
||||
expect(stored?.isLoading).toBe(false)
|
||||
expect(stored?.runForm?.canceled).toBe(true)
|
||||
// What every mid-turn save has to hold: no stored card renders a live form. A
|
||||
// save path added without settling would restore a Run that resolves nothing.
|
||||
expect(isActiveRunForm(stored!)).toBe(false)
|
||||
// The live card is untouched — the turn is still parked on it.
|
||||
expect(manager.displayMessages[0].isLoading).toBe(true)
|
||||
})
|
||||
|
||||
@@ -4318,8 +4318,8 @@ export const SESSION_PREVIEW_TOOL_NAMES = new Set([
|
||||
'list_artifact_versions'
|
||||
])
|
||||
|
||||
// Withheld from the side-panel chat: the argument form is a blocking card, and
|
||||
// sessions are the only surface that renders one.
|
||||
// Withheld from the side-panel chat, which is scoped to authoring what is open in the
|
||||
// editor rather than running deployed scripts. Both surfaces can render the form.
|
||||
const SESSION_ONLY_TOOL_NAMES = new Set(['run_script'])
|
||||
|
||||
/**
|
||||
|
||||
@@ -113,6 +113,22 @@ describe('conformArgsToSchema', () => {
|
||||
dropped: { undeclared: [], unshowable: ['name'] }
|
||||
})
|
||||
})
|
||||
|
||||
// MultiSelect maps over the value while rendering, so a non-array here throws and
|
||||
// takes the whole form with it — the user cannot even cancel what they were shown.
|
||||
it('drops a non-array in a dyn-multiselect slot', () => {
|
||||
const schema = {
|
||||
properties: { tenants: { type: 'object', format: 'dynmultiselect-list_tenants' } }
|
||||
}
|
||||
expect(conformArgsToSchema({ tenants: { evil: 1 } }, schema)).toMatchObject({
|
||||
args: {},
|
||||
dropped: { unshowable: ['tenants'] }
|
||||
})
|
||||
expect(conformArgsToSchema({ tenants: 'acme' }, schema)).toMatchObject({
|
||||
args: {},
|
||||
dropped: { unshowable: ['tenants'] }
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('enforceDisabledDefaults', () => {
|
||||
@@ -154,6 +170,18 @@ describe('enforceDisabledDefaults', () => {
|
||||
expect(resetKeys).toEqual(['top', 'cfg.force', 'list[0].mode', 'either.level'])
|
||||
})
|
||||
|
||||
// By value: a locked object default never matches by identity, so every run of such
|
||||
// a field reported a reset, and the caller was corrected for getting it right.
|
||||
it('reports no reset for an object default the caller already matched', () => {
|
||||
const objSchema = {
|
||||
properties: { opts: { type: 'object', disabled: true, default: { dry_run: true } } }
|
||||
}
|
||||
expect(enforceDisabledDefaults({ opts: { dry_run: true } }, objSchema).resetKeys).toEqual([])
|
||||
expect(enforceDisabledDefaults({ opts: { dry_run: false } }, objSchema).resetKeys).toEqual([
|
||||
'opts'
|
||||
])
|
||||
})
|
||||
|
||||
it('reports only the arguments it actually overwrote', () => {
|
||||
const { args, resetKeys } = enforceDisabledDefaults({ free: 'kept' }, schema)
|
||||
// The default still runs; the caller supplied nothing to overwrite, and one told
|
||||
|
||||
@@ -20,7 +20,9 @@ export function enforceDisabledDefaults(
|
||||
const result = mapMatchingArgs(args, schema.properties, isLockedProp, (value, prop, path) => {
|
||||
// An argument never supplied was not overwritten: the field shows the default
|
||||
// either way, and a caller told otherwise would try to correct what it never sent.
|
||||
if (value !== undefined && value !== prop.default) resetKeys.push(path)
|
||||
// By value, since a default can be an object or an array: identity would report
|
||||
// every run of such a field as overridden, the caller that got it right included.
|
||||
if (value !== undefined && !deepEqual(value, prop.default)) resetKeys.push(path)
|
||||
return prop.default
|
||||
})
|
||||
return { args: result, resetKeys }
|
||||
@@ -93,8 +95,18 @@ const DROP = Symbol('drop')
|
||||
/** Types `setInputCat` routes to a widget bound to a scalar. */
|
||||
const SCALAR_TYPES = new Set(['string', 'number', 'integer', 'boolean'])
|
||||
|
||||
/**
|
||||
* Declares an array even though its `type` says `object`, and the only slot where a
|
||||
* mismatch is worse than unreadable: `MultiSelect` maps over the value as it renders,
|
||||
* so anything else throws and takes the whole form down — Cancel with it.
|
||||
*/
|
||||
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 || typeof value !== 'object') return value
|
||||
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.
|
||||
|
||||
Reference in New Issue
Block a user