Stop offering a default on a field the state has made required
A schema-driven dialog read only the schema's own `required` list, so a field made required by `x-ui-required-when` - an MCP server's `url` once the catalog is off, its `name` - was drawn with "Use default" and could be saved empty. The inline editor already accounted for the conditional rule; this brings the dialog to the same answer, and the same rule now gates both the button and saving. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
074c8fb763
commit
eef5dfb3ed
|
|
@ -3,8 +3,10 @@ import {
|
|||
isFlowDataFieldPath
|
||||
} from './flow-data-schema-fields';
|
||||
import { parentGroupLabel } from './node-utility';
|
||||
import { validateFieldValue } from '@services/dialogs/node-settings-dialog';
|
||||
import {
|
||||
buildOrderedSchemaDisplay,
|
||||
buildSchemaObjectDialog,
|
||||
buildTemplatedRichContentParts,
|
||||
buildSchemaEditableFieldDefinitions,
|
||||
buildSchemaFieldViewModel,
|
||||
|
|
@ -720,4 +722,66 @@ describe('buildTemplatedRichContentParts', () => {
|
|||
|
||||
expect(buildTemplatedRichContentParts({ subject: ' ' }, 'subject', schema, splitParts)).toEqual([]);
|
||||
});
|
||||
|
||||
/**
|
||||
* The MCP server binding, reduced to the two properties that made the dialog misbehave: a
|
||||
* sourceType with a declared default, and a serverName that only the CATALOG state demands.
|
||||
*/
|
||||
const mcpBindingSchema = {
|
||||
type: 'object',
|
||||
required: [],
|
||||
properties: {
|
||||
sourceType: { type: 'string', enum: ['CATALOG', 'CUSTOM'], default: 'CATALOG', title: 'Source type' },
|
||||
serverName: {
|
||||
type: 'string',
|
||||
title: 'Server name',
|
||||
// The key the producer emits for @UiRequiredWhen(equalsAny = ...) is `in`, not `equalsAny`.
|
||||
'x-ui-required-when': { field: 'sourceType', in: ['CATALOG', ''] }
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
const dialogHooks = {
|
||||
schemaRoot: mcpBindingSchema,
|
||||
loadOptions: (propertySchema: Record<string, any>) => {
|
||||
const values: string[] = propertySchema['enum'] ?? [];
|
||||
return values.map((value) => ({ label: value, value }));
|
||||
}
|
||||
};
|
||||
|
||||
it('refuses to save a field that the current state requires and nobody filled', async () => {
|
||||
const dialog = await buildSchemaObjectDialog(mcpBindingSchema, 'MCP server', { sourceType: 'CATALOG' }, dialogHooks);
|
||||
const serverName = dialog.fields.find((field) => field.key === 'serverName')!;
|
||||
|
||||
// The gap this closes: an MCP server was saved with nothing selected, because the dialog only
|
||||
// ever read the schema's own `required` and this field is optional there.
|
||||
expect(serverName.required).toBe(true);
|
||||
expect(validateFieldValue(serverName, '')).toBe('Required');
|
||||
});
|
||||
|
||||
it('stops requiring it in the state that does not ask for it', async () => {
|
||||
const dialog = await buildSchemaObjectDialog(mcpBindingSchema, 'MCP server', { sourceType: 'CUSTOM' }, dialogHooks);
|
||||
const serverName = dialog.fields.find((field) => field.key === 'serverName')!;
|
||||
|
||||
expect(serverName.required).toBe(false);
|
||||
expect(validateFieldValue(serverName, '')).toBeNull();
|
||||
});
|
||||
|
||||
it('does not offer a default on a field the current state requires', async () => {
|
||||
const catalog = await buildSchemaObjectDialog(mcpBindingSchema, 'MCP server', { sourceType: 'CATALOG' }, dialogHooks);
|
||||
const custom = await buildSchemaObjectDialog(mcpBindingSchema, 'MCP server', { sourceType: 'CUSTOM' }, dialogHooks);
|
||||
|
||||
// "Use default" cleared serverName straight into an invalid state, towards a default that does
|
||||
// not exist. It belongs only where emptying the field is a legitimate answer.
|
||||
expect(catalog.fields.find((field) => field.key === 'serverName')!.defaultsWhenEmpty).toBe(false);
|
||||
expect(custom.fields.find((field) => field.key === 'serverName')!.defaultsWhenEmpty).toBe(true);
|
||||
});
|
||||
|
||||
it('names the default an empty field is using, when the schema declares one', async () => {
|
||||
const dialog = await buildSchemaObjectDialog(mcpBindingSchema, 'MCP server', {}, dialogHooks);
|
||||
const sourceType = dialog.fields.find((field) => field.key === 'sourceType')!;
|
||||
|
||||
expect(sourceType.defaultsWhenEmpty).toBe(true);
|
||||
expect(sourceType.defaultValue).toBe('CATALOG');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -913,8 +913,9 @@ export async function buildSchemaObjectDialog(
|
|||
|
||||
const fields: NodeSettingField[] = [];
|
||||
const initial: Record<string, string | boolean> = {};
|
||||
// Only to tell an optional field from a required one. Requiredness deliberately does not gate
|
||||
// saving here: an emptied required number still becomes 0, as it always has.
|
||||
// The schema's own requiredness, which still does not gate saving: an emptied required number
|
||||
// becomes 0 as it always has, and a blank @ConfigurableAsInput field means "from the input port".
|
||||
// What does gate saving is a x-ui-required-when rule satisfied by the current draft, below.
|
||||
const requiredKeys = new Set(Array.isArray(objectSchema?.['required']) ? objectSchema['required'] as string[] : []);
|
||||
|
||||
for (const { key, schema: propertySchema } of orderedSchemaPropertyEntries(objectSchema, schemaRoot)) {
|
||||
|
|
@ -927,6 +928,22 @@ export async function buildSchemaObjectDialog(
|
|||
);
|
||||
if (!visible) continue;
|
||||
|
||||
/*
|
||||
* Required as things stand, not just required in the abstract.
|
||||
*
|
||||
* <p>Most requiredness in these configurations is conditional: an MCP binding's serverName is
|
||||
* optional in the schema and mandatory the moment sourceType is CATALOG. Reading only the
|
||||
* schema's own `required` offered "Use default" on it, which cleared the field straight into an
|
||||
* invalid state - and towards a default that does not exist. Re-evaluated per render, so the
|
||||
* button appears and disappears as the field it depends on changes; onValuesChange rebuilds
|
||||
* this whole dialog from the draft, which is what makes that work.
|
||||
*/
|
||||
const requiredWhenRule = readUiConditionRule(propertySchema['x-ui-required-when']);
|
||||
const requiredByState = requiredWhenRule
|
||||
? evaluateUiConditionRule(requiredWhenRule, value, (fieldPath) => resolveSchemaPath(objectSchema, fieldPath))
|
||||
: false;
|
||||
const requiredNow = requiredKeys.has(key) || requiredByState;
|
||||
|
||||
if (hooks.dynamic?.isDynamic(propertySchema)) {
|
||||
const dynamicFields = await hooks.dynamic.buildFields(key, propertySchema, value);
|
||||
fields.push(...dynamicFields.fields);
|
||||
|
|
@ -963,8 +980,19 @@ export async function buildSchemaObjectDialog(
|
|||
minLength: isNumeric ? undefined : fieldUi.minLength,
|
||||
maxLength: isNumeric ? undefined : fieldUi.maxLength,
|
||||
pattern: isNumeric ? undefined : fieldUi.pattern,
|
||||
/*
|
||||
* Gated on the conditional rule alone, not on the schema's own `required`.
|
||||
*
|
||||
* <p>A statically required field is knowingly left blank in one case: a
|
||||
* @ConfigurableAsInput field - an LLM descriptor's model, say - stays required in the
|
||||
* published schema while blank means "comes from the node input", which is why saving was
|
||||
* never gated on it. A x-ui-required-when rule makes no such claim: it says this field must
|
||||
* be filled in the state the dialog is in, and an MCP binding with sourceType CATALOG and no
|
||||
* serverName was being saved as valid.
|
||||
*/
|
||||
required: requiredByState,
|
||||
// A checkbox is excluded because it has no empty state: false is a value, not an absence.
|
||||
defaultsWhenEmpty: !requiredKeys.has(key) && fieldType !== 'checkbox',
|
||||
defaultsWhenEmpty: !requiredNow && fieldType !== 'checkbox',
|
||||
defaultValue: propertySchema?.['default'] == null ? undefined : String(propertySchema['default']),
|
||||
readonly: !fieldUi.enabledWhen.every((rule) =>
|
||||
evaluateUiConditionRule(rule, value, (fieldPath) => resolveSchemaPath(objectSchema, fieldPath))
|
||||
|
|
|
|||
Loading…
Reference in New Issue