feat(apollo-react): guardrails component family under canvas - #1138
Open
apetraru-uipath wants to merge 14 commits into
Open
feat(apollo-react): guardrails component family under canvas#1138apetraru-uipath wants to merge 14 commits into
apetraru-uipath wants to merge 14 commits into
Conversation
apetraru-uipath
added
the
dev-packages
Contributor
Dependency License Review
License distribution
Excluded packages
|
apetraru-uipath
added
dev-packages
apetraru-uipath
force-pushed
the
feat/apollo-react-guardrails-family
branch
from
September 9, 2026 09:25
31a02e7 to
129e635
Compare
apetraru-uipath
changed the base branch from
feat/apollo-wind-guardrail-validator-form
to
main
September 9, 2026 09:29
apetraru-uipath
added
dev-packages
Contributor
📦 Dev Packages
|
This was referenced Sep 9, 2026
Draft
Contributor
|
Apollo Coded App preview deployments are ready.
|
Author
|
Note on the two red This PR's entire lockfile diff is 9 lines — Everything else is green — Build, Typecheck, Lint, and the test suites all pass. |
Contributor
📊 Coverage + size by packagePer-package coverage and bundle size on this PR. New-line coverage = of the source lines this PR adds or changes, the % hit by tests.
"Coverage" is each package's own |
Contributor
Storybook visual diffBaseline is the deployed main Storybook, so changes merged to main after this branch was last updated can also appear here. Logs Updated (PT): Sep 10, 2026, 06:56:56 AM |
This was referenced Sep 10, 2026
apetraru-uipath
force-pushed
the
feat/apollo-react-guardrails-family
branch
from
September 10, 2026 12:46
129e635 to
2ff0c35
Compare
… field Generic forms-engine enablers extracted from the guardrails work (apollo-ui#1107 review): controlled values/onValuesChange/errors/disableValidation/container props on MetadataForm (MetadataFormProps now exported), a string-list field type with tooltip/textarea/multiselect metadata additions, label association and custom-component ref handling in the field renderer, and a useWatch re-export so cross-package custom fields share the RHF context. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ling InfoTooltip moves out of the guardrails prototype into components/ui with its a11y test; select and textarea get aria-invalid error styling; the root barrel exposes the new forms/ui surface. The guardrails domain family itself moves to apollo-react (canvas) per the #1107 review decision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review follow-up. StringListField rendered the FormFieldError text but never marked the rows invalid, so the aria-invalid styling this PR adds to Textarea never activated and assistive tech got no invalid signal for the inputs. Every row now carries it — the error belongs to the list as a whole. Also adds the InfoTooltip storybook entry requested in review (default, label-adjacent, rich and long content), documenting that an ancestor TooltipProvider is required and that schema-driven forms get the trigger from the field `tooltip` metadata. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nts at Review follow-up: the label-adjacent story set htmlFor="blocked-phrases" with no such control, which is a dangling association and a misleading a11y example in the one story specifically about label placement. It now renders the Textarea the label names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review follow-up: rather than a near-duplicate label component, FormFieldLabel now takes `tooltip`/`tooltipAriaLabel` and renders the trigger after the required indicator. That removes the reason the field-renderer helper existed (it composed RequiredIndicator by hand purely to control that ordering), so the helper is gone and the 11 call sites use FormFieldLabel with its `required` prop directly. The tooltip is now available to every FormFieldLabel consumer, not just schema-driven forms; the InfoTooltip story shows that path as the idiomatic one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review follow-up, eight findings: - container='div' rendered schema submit actions as type="submit" with no owning <form>, so a click would submit whatever ancestor form the host embedded the fields in — the exact hazard div mode exists to avoid. Submit actions are now plain buttons wired to the form's own submit handler. - The tooltip trigger is a <button>, which <label>'s content model forbids as a descendant; it now renders beside the label in an inline wrapper, keeping the ref, htmlFor and styling on the label element. - text/email and number inputs never received aria-invalid, unlike the textarea/select paths, so their invalid styling and ARIA state never applied. - multiselect and file-upload had no label association: both now get the field id with htmlFor on the label (file upload also takes the field label as its dropzone name). - string-list never forwarded the controller's onBlur, so blur-mode validation and touched state never fired for it. - Textarea's new aria-invalid border is invisible in the future theme, which renders it borderless; it now carries the same invalid ring as Input. - The InfoTooltip story used the React UMD namespace without importing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second review pass, six findings — three of them gaps left by the previous round: - container='div' only neutralised the single-page submit button. Wizard and tabbed schemas still rendered type="submit", so the ancestor-form hazard survived for stepped forms; both step renderers now take the same handler. - file upload got the id but not htmlFor, so the label still was not associated with the control; it also never received aria-invalid, which FileUpload supports. - Select carried the same borderless-future-theme problem just fixed on Textarea, so its aria-invalid styling was invisible there. - A required multiselect or string-list accepted an empty array: zod treats [] as present, and the required branch only enforced non-empty for strings. Pre-existing for multiselect, and newly reachable for string-list. - Story copy used a spaced em dash, which the repo's Storybook rule forbids. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…olated Third review pass: - A schema with tooltip metadata crashed a bare MetadataForm: Radix throws without an ancestor TooltipProvider, and nothing mounted one — the story had to wrap manually, which was the tell. The form now supplies its own provider, but only for schemas that use tooltips, since a provider carries delay settings and an unconditional one would override a host's configuration. The story's manual wrapper is gone. - container='div' removed the <form> but not implicit submission: Enter in a single-line input still submits the host's ancestor form, contradicting the documented contract. Enter is now swallowed for inputs, leaving textareas (newlines) and buttons (activation) alone. - schema-serializer dropped every field property this PR added — tooltip, tooltipAriaLabel, minRows, maxLength, maxItems, emptyMessage, searchPlaceholder, addItemLabel, removeItemAriaLabel — so a round-trip through serializeSchema silently lost them. - The checkbox branch ignored tooltip metadata that every other field type honours. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fourth review pass, and this one hit the controlled-host seam itself. Initialization resets the form from schema.initialData asynchronously, so the reset lands after the values sync effect has already run. The prop reference is unchanged, so nothing re-applies it: a controlled host silently ended up displaying and submitting the schema's data instead of its own. Verified by probe before and after the fix. The sync now also depends on isInitialized, which is free when nothing differs thanks to the per-field deep-equal guard. StringListField is exported standalone and renders the Radix info tooltip when field.tooltip is set, so its contract now states the TooltipProvider requirement — MetadataForm keeps handling it for schema-driven usage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up review (BenGSchulz): the controlled-host seam routed around three features the schema contract already declared but never implemented. Repairing them serves every MetadataForm consumer, not just guardrails. - ValidationConfig.custom was typed, documented as a jsep expression and serialized, but the converter never read it. Now enforced. The expression is parsed once at schema-build time, because the evaluator returns false for a malformed expression exactly as for a legitimately failing one — a schema typo would otherwise make a field permanently unsubmittable, so an unparseable expression is simply not enforced. - plugin.onValueChange sat behind a mount-lifetime gate, so a plugin's first keystroke could be swallowed. It is now suppressed only while initialization's reset writes schema data. - Custom fields validated as z.any(), where required and minItems are no-ops. They can now declare a valueType, so the ordinary metadata constraints reach custom components. Also: one emptiness predicate shared by the resolver and the conditional- required superRefine, which previously disagreed — the resolver path missed whitespace-only strings. And the container='div' actions gate is gone; suppressing the action row is the schema's job via actions: [], which FormActions already honours. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up review (BenGSchulz): keep MetadataForm's contract as it was. Values live in react-hook-form, validation lives in the schema, and hosts interact through schema, plugins and remount. Removes `values`, `onValuesChange`, `errors`, `disableValidation` and `components`. They existed to route around three features the schema already declared but never implemented — all repaired in the preceding commit — so the seam bought nothing the intended path could not do. A second source of truth for values also means two things can disagree about what the user typed, and every consumer reimplements the reconciliation. `FormPlugin.components` stays honoured from the first render, which is what the `components` prop was compensating for. Tests and the string-list story move onto the plugin path, validating through `minItems` rather than a host-side reimplementation of the same rule, and forms/README.md now states the ownership contract once, naming NodePropertyPanel and ValidationPlugin as reference hosts. BREAKING CHANGE: MetadataForm no longer accepts `values`, `onValuesChange`, `errors`, `disableValidation` or `components`. Use schema validation, a FormPlugin (`onValueChange` / `context.form.setValue` / `setError`) and remount instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| @@ -1,6 +1,7 @@ | |||
| import { render, screen, waitFor, within } from '@testing-library/react'; | |||
| import { act, render, screen, waitFor, within } from '@testing-library/react'; | |||
apetraru-uipath
force-pushed
the
feat/apollo-react-guardrails-family
branch
from
September 10, 2026 12:51
2ff0c35 to
c9d8f20
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate issues remain across validation, accessibility, controlled state synchronization, and save behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Moves the Guardrails component family into apollo-react canvas components using Apollo Wind forms, shared Lingui catalogs, and new public exports.
Changes:
- Adds Guardrails builders, forms, selectors, fields, slots, utilities, tests, and stories.
- Extends Apollo Wind form and UI primitives.
- Adds canvas translations, package dependencies, exports, and accessibility test setup.
File summaries
| File | Summary |
|---|---|
pnpm-lock.yaml |
Locks updated dependencies. |
packages/apollo-wind/src/index.ts |
Exposes Wind primitives and form APIs. |
packages/apollo-wind/src/components/ui/textarea.tsx |
Adds textarea invalid-state styling. |
packages/apollo-wind/src/components/ui/select.tsx |
Adds select invalid-state styling. |
packages/apollo-wind/src/components/ui/info-tooltip.tsx |
Provides tooltip support. |
packages/apollo-wind/src/components/ui/info-tooltip.test.tsx |
Tests the info tooltip. |
packages/apollo-wind/src/components/ui/info-tooltip.stories.tsx |
Documents tooltip variants. |
packages/apollo-wind/src/components/ui/index.ts |
Updates UI exports. |
packages/apollo-wind/src/components/ui/form-field.tsx |
Provides form field behavior and errors. |
packages/apollo-wind/src/components/ui/form-field.test.tsx |
Tests form fields. |
packages/apollo-wind/src/components/forms/validation-converter.ts |
Converts form validation rules. |
packages/apollo-wind/src/components/forms/validation-converter.test.ts |
Tests validation conversion. |
packages/apollo-wind/src/components/forms/string-list-field.tsx |
Implements string-list editing. |
packages/apollo-wind/src/components/forms/schema-viewer.tsx |
Renders form schemas. |
packages/apollo-wind/src/components/forms/schema-serializer.ts |
Serializes form schemas. |
packages/apollo-wind/src/components/forms/schema-serializer.test.ts |
Tests schema serialization. |
packages/apollo-wind/src/components/forms/rules-engine.test.ts |
Tests form rules. |
packages/apollo-wind/src/components/forms/README.md |
Documents the forms API. |
packages/apollo-wind/src/components/forms/metadata-form.stories.tsx |
Documents MetadataForm usage. |
packages/apollo-wind/src/components/forms/index.ts |
Updates forms exports. |
packages/apollo-wind/src/components/forms/form-schema.ts |
Defines form schema types. |
packages/apollo-wind/src/components/forms/form-plugins.tsx |
Supports form plugins. |
packages/apollo-wind/src/components/forms/form-examples.tsx |
Provides form examples. |
packages/apollo-wind/src/components/forms/form-designer.tsx |
Provides the form designer. |
packages/apollo-wind/src/components/forms/demo-mocks.ts |
Provides demo mocks. |
packages/apollo-wind/src/components/forms/data-fetcher.ts |
Provides form data fetching. |
packages/apollo-wind/src/components/forms/data-fetcher.test.ts |
Tests data fetching. |
packages/apollo-wind/src/components/forms/custom-controls.stories.tsx |
Documents custom controls. |
packages/apollo-react/src/test/setup.ts |
Registers accessibility matchers. |
packages/apollo-react/src/i18n/index.ts |
Integrates localization exports. |
packages/apollo-react/src/canvas/locales/zh-TW.json |
Adds Traditional Chinese translations. |
packages/apollo-react/src/canvas/locales/zh-CN.json |
Adds Simplified Chinese translations. |
packages/apollo-react/src/canvas/locales/tr.json |
Adds Turkish translations. |
packages/apollo-react/src/canvas/locales/ro.json |
Adds Romanian translations. |
packages/apollo-react/src/canvas/locales/pt.json |
Adds Portuguese translations. |
packages/apollo-react/src/canvas/locales/pt-BR.json |
Adds Brazilian Portuguese translations. |
packages/apollo-react/src/canvas/locales/ko.json |
Adds Korean translations. |
packages/apollo-react/src/canvas/locales/ja.json |
Adds Japanese translations. |
packages/apollo-react/src/canvas/locales/fr.json |
Adds French translations. |
packages/apollo-react/src/canvas/locales/es.json |
Adds Spanish translations. |
packages/apollo-react/src/canvas/locales/es-MX.json |
Adds Mexican Spanish translations. |
packages/apollo-react/src/canvas/locales/en.json |
Adds English Guardrails messages. |
packages/apollo-react/src/canvas/locales/de.json |
Adds German translations. |
packages/apollo-react/src/canvas/components/index.ts |
Exports Guardrails components. |
packages/apollo-react/src/canvas/components/Guardrails/utils.ts |
Provides Guardrails validation utilities. |
packages/apollo-react/src/canvas/components/Guardrails/use-metadata-form-bridge.ts |
Bridges controlled values and errors. |
packages/apollo-react/src/canvas/components/Guardrails/types.ts |
Defines validator form types. |
packages/apollo-react/src/canvas/components/Guardrails/render-parameter-bridge.tsx |
Bridges parameter overrides. |
packages/apollo-react/src/canvas/components/Guardrails/index.ts |
Defines Guardrails exports. |
packages/apollo-react/src/canvas/components/Guardrails/guardrail-validator-form.tsx |
Implements the controlled validator form. |
packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.tsx |
Provides form layout shells. |
packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.test.tsx |
Tests form layouts. |
packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.stories.tsx |
Documents layout variants. |
packages/apollo-react/src/canvas/components/Guardrails/form-schema-builder.ts |
Builds MetadataForm schemas. |
packages/apollo-react/src/canvas/components/Guardrails/form-schema-builder.test.ts |
Tests schema generation. |
packages/apollo-react/src/canvas/components/Guardrails/components/parameter-label.tsx |
Renders parameter labels and tooltips. |
packages/apollo-react/src/canvas/components/Guardrails/components/mixed-scopes-banner.tsx |
Displays mixed-scope guidance. |
packages/apollo-react/src/canvas/components/Guardrails/components/mixed-scopes-banner.test.tsx |
Tests mixed-scope messaging. |
packages/apollo-react/src/canvas/components/Guardrails/components/map-enum-field.tsx |
Implements map threshold editing. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-banner.tsx |
Renders status notices. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-banner.test.tsx |
Tests status banners. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-scope-selector.tsx |
Implements scope and tool selection. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-scope-selector.test.tsx |
Tests scope selection. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-chip.tsx |
Defines Guardrails toggle chips. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-chip.test.tsx |
Tests chip behavior. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-action-section.tsx |
Renders action-specific controls. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-action-section.test.tsx |
Tests action controls. |
packages/apollo-react/src/canvas/components/Guardrails/components/field-shell.tsx |
Styles composite field controls. |
packages/apollo-react/src/canvas/components/Guardrails/components/field-shell.test.tsx |
Tests field-shell styling. |
packages/apollo-react/src/canvas/components/Guardrails/components/enum-list-chips-field.tsx |
Implements enum-list chip editing. |
packages/apollo-react/src/canvas/components/Guardrails/builder-utils.ts |
Provides builder initialization helpers. |
packages/apollo-react/src/canvas/components/Guardrails/builder-utils.test.ts |
Tests builder utilities. |
packages/apollo-react/src/canvas/components/Guardrails/builder-types.ts |
Defines builder types and slots. |
packages/apollo-react/package.json |
Adds exports and dependencies. |
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (10)
packages/apollo-react/src/canvas/components/Guardrails/README.md:169
- This section documents
MetadataFormprops (values,onValuesChange,errors,disableValidation, andcomponents) that are not part of the current exportedMetadataFormProps; the adapter actually passes apluginsbridge andcontainer="div". Consumers following this documentation cannot implement the described integration. Update the stack description to match the plugin-based API (or add and export the documented props before publishing).
`GuardrailValidatorForm` is not a form renderer of its own: internally it is
`buildGuardrailFormSchema(definitions, labels)` + the package's `MetadataForm`
(`components/forms/`: `FormSchema` → `MetadataForm` → `field-renderer`), mounted through the
controlled-host seam (`values` / `onValuesChange` / `errors` / `disableValidation` /
`container="div"` / synchronous `components`). The public contract above is the adapter
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-scope-selector.tsx:114
- These
<Label>elements are not associated with a form control: the actual controls are buttons inside the followingFieldShell, so the scope/tool heading is neither clickable nor announced as the group name. Render each set as afieldset/legend(or a labelled group witharia-labelledby) and expose the group’s invalid state.
<Label>
{labels.scopesLabel}
<RequiredIndicator />
</Label>
<FieldShell invalid={Boolean(errors?.scopes)}>
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-scope-selector.tsx:103
- Filtering
selectedToolsdown toavailableToolNameshides stale targets without removing them fromselector.matchNames. If a tool is removed while another remains, the user cannot see or deselect the stale name and the builder still saves it; reconcilematchNameswhen the available tool list changes or render stale targets for removal.
const targetedTools = useMemo(
() => selectedTools.filter((name) => availableToolNames.includes(name)),
[selectedTools, availableToolNames]
);
const addableTools = useMemo(
() => availableToolNames.filter((name) => !selectedTools.includes(name)),
[availableToolNames, selectedTools]
packages/apollo-react/src/canvas/components/Guardrails/components/mixed-scopes-banner.tsx:29
- Tool names are host-supplied, so a tool named
Agent(or another scope value) collides with the scope key in this same<ul>. React will warn about duplicate keys and can reconcile the list incorrectly; prefix keys by item category.
{otherAppliedScopes.tools.map((tool) => (
<li key={tool}>{tool}</li>
))}
packages/apollo-react/src/canvas/components/Guardrails/guardrail-builder.tsx:364
- The builder forwards
errors.parametersbut does not passGuardrailValidatorForm'sonClearError, and it exposes no parameter-change callback. Consequently, once a host parameter error is supplied, editing that parameter cannot clear it from this whole-screen API; the host has no way to observe the edit andhasHostErrorscontinues to block Save. Expose/forward a parameter-error clear or change callback, or define an internal clearing policy.
<GuardrailValidatorForm
parameterDefinitions={definition.parameters}
parameters={formData.validatorParameters}
onChange={(params) => updateField('validatorParameters', params)}
errors={validatorFormErrors}
renderParameter={renderParameter}
packages/apollo-react/src/canvas/components/Guardrails/guardrail-builder.tsx:255
- Empty host messages are treated as errors here because the predicate only checks whether the field or parameter map has a key. Clearing
errors.nameorerrors.parameters[id]to''therefore leaves Save disabled while no message is rendered; treat empty messages as absent, consistently with the validator bridge.
const hasHostErrors = Boolean(
hostErrors &&
Object.values(hostErrors).some(
(v) => v !== undefined && (typeof v !== 'object' || Object.keys(v).length > 0)
)
packages/apollo-react/src/canvas/components/Guardrails/guardrail-builder.tsx:245
- The merge treats any defined host value as an override, so
name: ''orparameters: { id: '' }replaces an existing internal error with an empty message. After a failed Save, clearing a host error can therefore leave the form blocked with no visible error; merge only non-empty host messages (including nested parameter entries).
for (const [key, value] of Object.entries(hostErrors)) {
if (value !== undefined) {
(base as Record<string, unknown>)[key] =
key === 'parameters'
? { ...base.parameters, ...(value as Record<string, string>) }
: value;
}
packages/apollo-react/src/canvas/components/Guardrails/guardrail-validator-form.tsx:191
- This adapter is documented as validation-free, but the schema passed here still contains
required/min/maxvalidation anduseMetadataFormBridgeselectsonChange;MetadataFormtherefore runs its resolver and emits its own generic errors alongside host-providederrors. That breaks the controlled contract when a user clears a required field or enters an out-of-range value. Pass the intended validation-disable option through the MetadataForm seam, or omit these validation rules before mounting this form.
packages/apollo-react/src/canvas/components/Guardrails/use-metadata-form-bridge.ts:78 - When a controlled host removes a defined parameter from
parameters,valuessimply omits that key, but this effect only writes present entries. React Hook Form therefore retains the old value and the UI can show or re-emit a parameter the host deleted; reconcile missing defined fields as well as syncing present values.
packages/apollo-react/src/canvas/components/Guardrails/utils.ts:170 - Required
map-enumvalidation checks the raw map keys, but those keys can be stale relative to thekeySource.syncMapEnumParameterslater prunes the map to the current selection; therefore a non-empty default/stale map plus an empty source selection is reported as valid here and can be persisted as an empty required map. Validate the reconciled map, or derive the effective key set fromkeySourcebefore deciding that the parameter is filled.
- Files reviewed: 86/87 changed files
- Comments generated: 8
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+179
to
+182
| <Label> | ||
| {recipientTypeLabels[displayedRecipientType] ?? labels.recipientFallbackLabel} | ||
| <RequiredIndicator /> | ||
| </Label> |
Comment on lines
+69
to
+73
| <Input | ||
| aria-label={`${paramDef.label}: ${sourceDef?.optionLabels?.[key] ?? key}`} | ||
| type="number" | ||
| value={currentMap[key] ?? defaults[key] ?? paramDef.min ?? 0} | ||
| onChange={(e) => handleThresholdChange(key, Number.parseFloat(e.target.value) || 0)} |
Comment on lines
+19
to
+35
| const content = ( | ||
| <> | ||
| {paramDef.label} | ||
| {paramDef.required && <RequiredIndicator />} | ||
| {paramDef.tooltip && ( | ||
| <InfoTooltip content={paramDef.tooltip} aria-label={labels.moreInformation} /> | ||
| )} | ||
| </> | ||
| ); | ||
| if (asTextHeader) { | ||
| return ( | ||
| <div data-slot="guardrail-parameter-label" className="text-xs font-medium text-foreground"> | ||
| {content} | ||
| </div> | ||
| ); | ||
| } | ||
| return <Label htmlFor={htmlFor}>{content}</Label>; |
Comment on lines
+83
to
+85
| tooltip: def.tooltip, | ||
| tooltipAriaLabel: labels.moreInformation, | ||
| validation: buildFieldValidation(def), |
Comment on lines
+213
to
+217
| const emptyParamIds = getRequiredEmptyParameterIds( | ||
| definition.parameters, | ||
| formData.validatorParameters | ||
| ); | ||
| if (emptyParamIds.length > 0) { |
| ? { | ||
| label: labels.saveAsNew, | ||
| onClick: handleSaveAsNew, | ||
| disabled: !isDefinitionAvailable, |
Comment on lines
+70
to
+72
| for (const [name, value] of Object.entries(values)) { | ||
| if (!Object.is(current[name], value)) { | ||
| form.setValue(name, value as never); |
Comment on lines
+118
to
+120
| for (const paramDef of definitions) { | ||
| if (paramDef.type !== 'number') continue; | ||
| if (paramDef.min == null && paramDef.max == null) continue; |
Awaiting a plugin onFormInit that returns void still defers setIsInitialized into a microtask, so it lands outside React's act() scope and every host test that renders synchronously emits an act warning — enough to fail a suite that treats console output as an error. Only promise-returning hooks are awaited now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Moves the guardrails family out of apollo-wind's custom/ prototype shelf into apollo-react per the #1107 review decision: canvas-adjacent (MUI-free, built on wind primitives and the forms/ MetadataForm engine), exported from the canvas components barrel plus a narrow ./canvas/guardrails subpath. Strings move to lingui: useSafeLingui labels-hooks with explicit guardrails.* ids replace the wind-local catalogs and loader; the 60 keys ship translated in the shared canvas catalog for 13 locales (ru falls back to English per key). Localized templates that cross into plain-string APIs are ICU messages formatted with sentinel values, preserving the {{token}} convention. The family's Tailwind classes ride the existing tailwind.canvas.css scan; adds class-variance-authority (dep) and jest-axe (dev, matcher registered in the shared test setup). Also silences Radix's aria-describedby warning on the description-less builder dialog. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ugin seam Follow-up review (BenGSchulz). `GuardrailValidatorForm` keeps its controlled public contract — `parameters`/`onChange` plus host-owned `errors`/`onClearError` are unchanged for both consumers — but it no longer asks `MetadataForm` for controlled props, which that PR removes. The translation now lives in one named place, `useMetadataFormBridge`: a `FormPlugin` that declares the custom components (honoured from the first paint), reports user edits through `onValueChange`, pushes host values in per field via `context.form.setValue` (deep-equal guarded, and suppressed while syncing so a sync-in never echoes back as a user edit), and applies host errors as `type: 'external'`. Keeping it in one hook is the point: `context.form` is an unlabelled backdoor, and a reviewer who greps for a controlled prop would never find a scattered `setValue`. Also fixes a live bug Ben spotted: number parameters carried `min`/`max` only as DOM attributes, which browsers enforce on native form submission — something this form never does, since the host owns saving. An out-of-range guardrail was therefore saveable. The range is now declared in the field's `validation` (with `mode: 'onChange'`, or it would never be evaluated), and `getOutOfRangeParameterIds` gives hosts the same shape they already use for `getRequiredEmptyParameterIds` so Save can be gated on it. One test expectation moved: the enum-list combobox is now named by its visible field label rather than its placeholder, because the renderer associates them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
apetraru-uipath
force-pushed
the
feat/apollo-react-guardrails-family
branch
from
September 10, 2026 13:09
c9d8f20 to
d658731
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved review findings remain, including controlled-value synchronization, validation, save-gating, accessibility, and documentation issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (10)
packages/apollo-react/src/canvas/components/Guardrails/README.md:169
- This section documents
MetadataFormprops (values,onValuesChange,errors, anddisableValidation) that are not present in the currentMetadataFormProps; this package actually adapts the plugin API throughuseMetadataFormBridge. Published documentation therefore describes a shared API consumers cannot use. Update this section to match the shipped bridge or include the claimed forms-layer surface.
`GuardrailValidatorForm` is not a form renderer of its own: internally it is
`buildGuardrailFormSchema(definitions, labels)` + the package's `MetadataForm`
(`components/forms/`: `FormSchema` → `MetadataForm` → `field-renderer`), mounted through the
controlled-host seam (`values` / `onValuesChange` / `errors` / `disableValidation` /
`container="div"` / synchronous `components`). The public contract above is the adapter
packages/apollo-react/src/canvas/components/Guardrails/components/escalate-action-fields.tsx:182
- This recipient
Labelis not linked to either built-in input: it has nohtmlFor, and the inputs in both fallback branches have no matching id. The visible field name is therefore not programmatically associated with the control. Add a stable id/htmlFor pair, and define how a custom recipient slot receives the accessible label.
<Label>
{recipientTypeLabels[displayedRecipientType] ?? labels.recipientFallbackLabel}
<RequiredIndicator />
</Label>
packages/apollo-react/src/canvas/components/Guardrails/components/escalate-action-fields.tsx:257
- When no
renderAppPickerslot is supplied, the builder still reportsactionAppas a required error and blocks Save, but this fallback renders only the informational alert. After a failed save there is no error message explaining why the action cannot be persisted. Rendererrors?.actionApphere (or make the unavailable action unselectable).
<Alert variant="info">
<Info />
<AlertDescription>{labels.appPickerUnavailable}</AlertDescription>
</Alert>
packages/apollo-react/src/canvas/components/Guardrails/components/map-enum-field.tsx:77
- Host/resolver errors are rendered below the map, but these numeric controls never receive
aria-invalid, unlike the standard and string-list fields. A required/error state is therefore not exposed to assistive technology or the input's error styling. Pass the error state through to each input.
<Input
aria-label={`${paramDef.label}: ${sourceDef?.optionLabels?.[key] ?? key}`}
type="number"
value={currentMap[key] ?? defaults[key] ?? paramDef.min ?? 0}
onChange={(e) => handleThresholdChange(key, Number.parseFloat(e.target.value) || 0)}
min={paramDef.min}
max={paramDef.max}
step={paramDef.step}
className="flex-1"
packages/apollo-react/src/canvas/components/Guardrails/components/parameter-label.tsx:25
- When a parameter has a tooltip, this places
InfoTooltip's real<button>inside the associated<Label>. A labelable descendant can activate the editor when the icon is clicked and violates the label content model, so the tooltip trigger is not an independent accessible control. Keep only the text and required indicator insideLabeland render the tooltip as a sibling, asFormFieldLabeldoes.
{paramDef.label}
{paramDef.required && <RequiredIndicator />}
{paramDef.tooltip && (
<InfoTooltip content={paramDef.tooltip} aria-label={labels.moreInformation} />
)}
packages/apollo-react/src/canvas/components/Guardrails/form-schema-builder.ts:85
- These fields still carry
validation: { required, min, max }, while the bridge setsschemaMode: 'onChange'andMetadataFormalways installs a Zod resolver. Clearing a required value or entering an out-of-range number therefore produces an internal error even when the hosterrorsmap is empty, contradicting the documented validation-free, host-owned contract and potentially leaving client errors visible after the host clears its error. Remove these validation constraints for this adapter or add and use the forms layer's actual disable-validation path.
const base = {
name: def.id,
label: def.label,
tooltip: def.tooltip,
tooltipAriaLabel: labels.moreInformation,
validation: buildFieldValidation(def),
packages/apollo-react/src/canvas/components/Guardrails/guardrail-builder.tsx:415
hostSaveDisabledis applied only to the primary Save button; the secondary Save as new action is disabled only when the definition is unavailable. When a host usessaveDisabledfor an async gate, Save as new remains clickable andhandleSaveAsNewbypasses that gate. Include the host gate in the secondary action's disabled state as well.
const secondaryAction = onSaveAsNew
? {
label: labels.saveAsNew,
onClick: handleSaveAsNew,
disabled: !isDefinitionAvailable,
}
packages/apollo-react/src/canvas/components/Guardrails/guardrail-builder.tsx:217
- This Save gate only checks required parameters; it never calls
getOutOfRangeParameterIds. As a result, editing a numeric parameter past itsmin/maxcan still leaveisValidtrue andhandleSaveserializes the invalid value. The MetadataForm field error is not connected to this builder's Save gate, so range failures need to be included here (with a localized message) or explicitly gate Save via the exported predicate.
const emptyParamIds = getRequiredEmptyParameterIds(
definition.parameters,
formData.validatorParameters
);
if (emptyParamIds.length > 0) {
packages/apollo-react/src/canvas/components/Guardrails/utils.ts:120
GuardrailParameterDefinitiondocumentsmin/maxformap-enum, and each map row is rendered as a numeric input with those bounds, but this save-time helper immediately skips every non-numberdefinition. Even if the builder starts using this helper, out-of-range map thresholds would still pass validation and be persisted. Validate the numeric entries ofmap-enumvalues (or remove those constraints) and cover that path.
packages/apollo-wind/src/components/forms/validation-converter.ts:56- The required string check uses
.min(1), so a whitespace-only value passes even though this file'sisEmptyFieldValuedefinition treats trimmed whitespace as empty and conditional-required validation uses that helper. Use the shared emptiness predicate for static required strings as well.
- Files reviewed: 86/87 changed files
- Comments generated: 3
- Review effort level: Lite
Comment on lines
+67
to
+73
| const current = form.getValues(); | ||
| syncingRef.current = true; | ||
| try { | ||
| for (const [name, value] of Object.entries(values)) { | ||
| if (!Object.is(current[name], value)) { | ||
| form.setValue(name, value as never); | ||
| } |
Comment on lines
+359
to
+363
| <GuardrailValidatorForm | ||
| parameterDefinitions={definition.parameters} | ||
| parameters={formData.validatorParameters} | ||
| onChange={(params) => updateField('validatorParameters', params)} | ||
| errors={validatorFormErrors} |
Comment on lines
+187
to
+190
| <MetadataForm | ||
| schema={useMemo(() => ({ ...schema, mode: schemaMode }), [schema, schemaMode])} | ||
| plugins={plugins} | ||
| container="div" |
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved moderate issues affect accessibility, validation, localization, and save behavior.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (13)
packages/apollo-react/src/canvas/components/Guardrails/components/escalate-action-fields.tsx:182
- The built-in escalation fallback inputs are not associated with the visible recipient label: this
Labelhas nohtmlFor, and the fallback inputs in both the searchable and static branches have no matchingidor accessible name. Without a host slot, screen readers therefore expose an unlabeled recipient textbox. Give each fallback control a unique id and connect the label (or provide an equivalent explicit accessible name).
<Label>
{recipientTypeLabels[displayedRecipientType] ?? labels.recipientFallbackLabel}
<RequiredIndicator />
</Label>
packages/apollo-react/src/canvas/components/Guardrails/components/map-enum-field.tsx:73
- This custom numeric editor receives the field error but does not pass it to the
<Input>or setaria-invalid. The error text is rendered later, yet range/host errors leave each threshold control styled and exposed as valid, unlike the standard MetadataForm number renderer. Propagate the invalid state (and its error association) to the input.
<Input
aria-label={`${paramDef.label}: ${sourceDef?.optionLabels?.[key] ?? key}`}
type="number"
value={currentMap[key] ?? defaults[key] ?? paramDef.min ?? 0}
onChange={(e) => handleThresholdChange(key, Number.parseFloat(e.target.value) || 0)}
packages/apollo-react/src/canvas/components/Guardrails/components/parameter-label.tsx:24
- When
paramDef.tooltipis present, this putsInfoTooltip's real<button>inside a<label>. Labelable descendants make the label invalid and make clicking the icon ambiguously activate the associated control; the sharedFormFieldLabeldeliberately places the tooltip beside the label for this reason. Use a sibling wrapper for the label and tooltip, preservinghtmlFor.
{paramDef.tooltip && (
<InfoTooltip content={paramDef.tooltip} aria-label={labels.moreInformation} />
packages/apollo-react/src/canvas/components/Guardrails/form-schema-builder.ts:177
- The large
enum-listpath only supplies the localized trigger placeholder.MultiSelecttherefore falls back to its EnglishSearch...,No items found.,Clear all (...), and selected-itemRemove ...copy when this form is used under a non-English canvas catalog, even though this component's contract says its chrome strings are localized through Lingui. Add the corresponding localized labels and pass them through (including the generic control props/API needed for the remove label).
return {
...base,
type: 'multiselect',
placeholder: labels.enumListPlaceholder,
options: (def.options ?? []).map((opt) => ({
value: opt,
label: def.optionLabels?.[opt] ?? opt,
})),
defaultValue: selected,
packages/apollo-react/src/canvas/components/Guardrails/guardrail-builder.tsx:221
- This save-validation path only adds
getRequiredEmptyParameterIdstointernalErrors.handleSavethen relies on that object, so a scalar number outside its definition'smin/maxcan still reachonSaveeven though the editor exposes those constraints. Include the range predicate in the builder's gate with a localized message, or make the builder explicitly require those host errors.
const emptyParamIds = getRequiredEmptyParameterIds(
definition.parameters,
formData.validatorParameters
);
if (emptyParamIds.length > 0) {
e.parameters = Object.fromEntries(
emptyParamIds.map((id) => [id, labels.parameterRequiredError])
);
}
packages/apollo-react/src/canvas/components/Guardrails/guardrail-builder.tsx:415
- The extra
saveDisabledgate is applied to the primary button at line 428, but the secondary action only checksisDefinitionAvailable, andhandleSaveAsNewdoes not checkhostSaveDisabled. When a host disables saving during an async slot operation, users can still bypass that gate with “Save as new”. Include the host gate in this secondary action as well.
const secondaryAction = onSaveAsNew
? {
label: labels.saveAsNew,
onClick: handleSaveAsNew,
disabled: !isDefinitionAvailable,
}
packages/apollo-react/src/canvas/components/Guardrails/guardrail-builder.tsx:364
GuardrailValidatorFormonly clears host parameter errors when itsonClearErrorcallback is provided, but the builder does not expose or pass one here. A server error supplied througherrors.parameterstherefore remains indisplayErrorsand keepshasHostErrorstrue/Save blocked even after the user edits the parameter; this builder has no value-change callback for the host to clear it. Add a builder-level clear callback or an explicit local policy for clearing parameter errors on edit.
<GuardrailValidatorForm
parameterDefinitions={definition.parameters}
parameters={formData.validatorParameters}
onChange={(params) => updateField('validatorParameters', params)}
errors={validatorFormErrors}
renderParameter={renderParameter}
packages/apollo-react/src/canvas/components/Guardrails/guardrail-builder.tsx:216
- Required validation runs on the unsynchronized parameter array, but
guardrailResultlater prunesmap-enumkeys to the currentkeySourceselection. If a required map has a now-deselected stale key while its source list is optional, this check passes and Save persists the synchronized empty map. Validate the same synchronized values that will be saved.
const emptyParamIds = getRequiredEmptyParameterIds(
definition.parameters,
formData.validatorParameters
);
packages/apollo-react/src/canvas/components/Guardrails/guardrail-validator-form.tsx:191
- This schema marks required/range constraints, and
schemaModeis forced toonChange, soMetadataForm's Zod resolver still produces its own validation errors. That breaks the documented validation-free controlled contract and can overwrite or clear host-suppliederrorswhen another field changes. Disable the resolver for this adapter (or omit these constraints) while retaining the host-error bridge.
packages/apollo-react/src/canvas/components/Guardrails/use-metadata-form-bridge.ts:72 - The comment promises a deep-equality guard, but
Object.isonly compares references. Controlled echoes commonly recreate array/object values fortext-list,enum-list, and map parameters, so equivalent values still callsetValueon every echo, causing needless RHF/watch rerenders and potentially disturbing focused editors. Compare values structurally before synchronizing.
packages/apollo-react/src/canvas/components/Guardrails/utils.ts:129 GuardrailParameterDefinitiondocumentsmin/maxformap-enum, but this helper only examinesparamDef.type === 'number'.MapEnumField's HTML attributes do not validate programmatic or controlled values, so a map entry such as2withmax: 1survivessyncMapEnumParametersand can be saved. Validate each map value against the definition bounds as well.
packages/apollo-wind/src/components/forms/validation-converter.ts:47customValueTypeis threaded into string and array validation, butapplyNumberConstraintsstill checks onlyfieldTypethroughisNumberType. Atype: 'custom'field withvalueType: 'number'therefore ignoresmin,max,integer,positive, andnegative, despite the metadata contract saying normal constraints apply to declared custom shapes. Pass the custom value type through the number path and cover it with a test.
packages/apollo-wind/src/components/forms/validation-converter.ts:57- The static required-string path uses
min(1), so a whitespace-only value passes, while the newly sharedisEmptyFieldValuepredicate explicitly treats trimmed whitespace as empty and the dynamic-required path uses that predicate. This makes required validation depend on whether the field is static or rule-driven; apply the same trimmed emptiness check to the static schema and add a whitespace regression test.
- Files reviewed: 86/87 changed files
- Comments generated: 0 new
- Review effort level: Lite
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed?
The guardrails component family moves from
apollo-wind/custom/guardrailstoapollo-react, per the #1107 review decision and the design doc §7.4 (revised 2026-09-09): canvas-adjacent placement atsrc/canvas/components/Guardrails/, MUI-free, built entirely on@uipath/apollo-windprimitives + itsforms/MetadataForm engine, strings on lingui.Placement & API
src/canvas/components/Guardrails/—GuardrailBuilder(whole Add/Edit screen),GuardrailFormLayout,GuardrailValidatorForm, escalation/parameter host slots, save-time companions, type surface — public API unchanged from the wind revision except thelocaleprop (dropped; see i18n)../canvas/guardrailssubpath (dist/canvas/components/Guardrails/index.*) — consumers with exports-map-blind test runners (Agents' jest) map one file instead of the whole./canvasbarrel.@uipath/apollo-windroot barrel (theNodePropertyPanelidiom).useWatchcomes from wind's re-export so custom fields share MetadataForm's react-hook-form context across the package boundary.i18n: lingui, canvas catalog
loadGuardrailMessagesloader +.gitignorenegation) is deleted. Strings useuseSafeLinguiwith explicitguardrails.*ids and English defaults, viauseGuardrailFormLabels/useGuardrailBuilderLabelshooks (theuseStageNodeLabelspattern). Thelabelsper-string override props survive unchanged.src/canvas/locales/*.json, 60 ids × 13 locales;rudeliberately falls back to English per key). Hosts already mountingApI18nProvider component="canvas"(Flow) get translations with zero changes; without a provider the components render English.formatTemplate) are ICU messages formatted with sentinel values reifying{token}→{{token}}— translators see standard ICU, the template convention survives (seeTEMPLATE_TOKENS).CSS
tailwind.canvas.cssalready@source-scanssrc/canvas/**and wind's dist, so the family's classes ship in the existing compiled sheet. Shadow-DOM hosts injectcanvas/styles/tailwind.canvas.css?inline.Package plumbing
class-variance-authority(GuardrailChip variants); devDeps:jest-axe+@types/jest-axe, with the matcher registered insrc/test/setup.ts(first a11y assertions in this package).Consumers
@uipath/apollo-react/canvas/guardrailsbehind their existing feature flags; both pin this PR'sdev-packagespreview plus feat(apollo-wind): metadata-form controlled-host seam, string-list field, InfoTooltip #1107's wind preview (a preview of this package pins released wind, so hosts must override wind resolution to the wind preview until both release).How has this been tested?
tscclean, biome clean,lingui compileclean (the 3 pre-existingcanvas.json_value_panel.item_countplural errors predate this PR),pnpm buildproducesdist/canvas/components/Guardrails/*(dual format) and the family's classes intailwind.canvas.css.🤖 Generated with Claude Code