feat(apollo-react): shared guardrail definitions layer and useGuardrailDefinitions [AL-574] - #1139
andreizdrali-uipath wants to merge 8 commits into
Conversation
|
Apollo Coded App preview deployments are ready.
|
Dependency License Review
License distribution
Excluded packages
|
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed behavioral bugs in the new code (unexpected refetch behavior and render-phase state updates) that should be fixed before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds a shared “guardrail definitions” layer under packages/apollo-react/src/canvas/components/Guardrails/ that parses the /api/execution/guardrails/definitions payload, enriches it with canonical (Lingui-backed) display copy, and exposes a useGuardrailDefinitions hook as the seam between host transport and GuardrailBuilder. It also extends apollo-wind’s metadata forms to support tooltips and a new string-list field type used by guardrail parameter editors.
Changes:
- Introduces wire types + zod-based non-throwing parsing, pure enrichment, and a
useGuardrailDefinitionshook for fetching/composing guardrail definitions. - Moves canonical validator copy into the canvas Lingui catalog and adds parity tests against Flow/Agents baselines.
- Enhances
apollo-windforms/UI withInfoTooltip,string-listfield support, andaria-invalidstyling for select/textarea.
File summaries
| File | Description |
|---|---|
| pnpm-lock.yaml | Locks new deps added for guardrails UI and a11y testing. |
| packages/apollo-wind/src/index.ts | Re-exports new forms/types (MetadataFormProps, useWatch, StringListField*) and InfoTooltip. |
| packages/apollo-wind/src/components/ui/textarea.tsx | Adds aria-invalid error styling to textarea. |
| packages/apollo-wind/src/components/ui/select.tsx | Adds aria-invalid error styling to select trigger. |
| packages/apollo-wind/src/components/ui/info-tooltip.tsx | Adds reusable info-icon tooltip component for form labels. |
| packages/apollo-wind/src/components/ui/info-tooltip.test.tsx | Adds a11y + behavior tests for InfoTooltip. |
| packages/apollo-wind/src/components/ui/index.ts | Exports info-tooltip (and reorders a couple exports). |
| packages/apollo-wind/src/components/forms/validation-converter.ts | Extends schema conversion to treat string-list as an array type. |
| packages/apollo-wind/src/components/forms/string-list-field.tsx | Implements the new string-list field editor and formatTemplate helper. |
| packages/apollo-wind/src/components/forms/metadata-form.stories.tsx | Adds story demonstrating string-list + tooltip + controlled-host seam. |
| packages/apollo-wind/src/components/forms/index.ts | Exposes new forms APIs (controlled seam types, string-list exports, useWatch). |
| packages/apollo-wind/src/components/forms/form-schema.ts | Adds tooltip metadata, textarea constraints, multiselect copy overrides, and string-list field metadata/type. |
| packages/apollo-wind/src/components/forms/field-renderer.tsx | Renders required indicator + optional tooltip, wires htmlFor/id, and passes aria-invalid to select/textarea/multiselect. |
| packages/apollo-react/src/test/setup.ts | Registers jest-axe matchers for Vitest suites. |
| packages/apollo-react/src/i18n/index.ts | Exports getPreImportedMessages helper for hosts merging catalogs. |
| packages/apollo-react/src/canvas/locales/en.json | Adds guardrails chrome strings + canonical validator/parameter/option copy (English). |
| packages/apollo-react/src/canvas/components/index.ts | Exports the Guardrails canvas family from the canvas components barrel. |
| packages/apollo-react/src/canvas/components/Guardrails/utils.ts | Adds parameter seeding/sync/validation helpers for guardrail parameters. |
| packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts | Adds useGuardrailDefinitions hook (fetch + parse + enrich + refetch). |
| packages/apollo-react/src/canvas/components/Guardrails/types.ts | Defines guardrail parameter and form prop types for the validator editor surface. |
| packages/apollo-react/src/canvas/components/Guardrails/render-parameter-bridge.tsx | Bridges host renderParameter overrides into MetadataForm custom components via context. |
| packages/apollo-react/src/canvas/components/Guardrails/index.ts | Public exports for the guardrails family, including the new definitions layer APIs. |
| packages/apollo-react/src/canvas/components/Guardrails/guardrail-validator-form.tsx | Implements validator parameter form using MetadataForm + guardrail-owned custom fields. |
| packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.tsx | Adds shared modal/inline layout wrapper for guardrail builder forms. |
| packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.test.tsx | Adds behavior + a11y tests for the shared form layout. |
| packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.stories.tsx | Adds Storybook examples for the shared form layout modes. |
| packages/apollo-react/src/canvas/components/Guardrails/form-schema-builder.ts | Builds MetadataForm schemas for guardrail parameters + coercion helper. |
| packages/apollo-react/src/canvas/components/Guardrails/form-schema-builder.test.ts | Tests schema mapping/coercion rules for guardrail parameter definitions. |
| packages/apollo-react/src/canvas/components/Guardrails/definitions-wire.ts | Adds hand-written wire types for the definitions endpoint payload. |
| packages/apollo-react/src/canvas/components/Guardrails/definitions-parse.ts | Adds zod validation + non-throwing parse result and issue reporting. |
| packages/apollo-react/src/canvas/components/Guardrails/definitions-parse.test.ts | Tests parsing guarantees + zod boundary constraints. |
| packages/apollo-react/src/canvas/components/Guardrails/definitions-parity.test.ts | Ensures canonical English matches Flow/Agents baselines and declares divergences. |
| packages/apollo-react/src/canvas/components/Guardrails/definitions-enrich.ts | Adds pure enrichment (copy resolution + parameter shaping + folder metadata helper). |
| packages/apollo-react/src/canvas/components/Guardrails/definitions-copy.test.ts | Tests copy table, message id conventions, and catalog parity. |
| packages/apollo-react/src/canvas/components/Guardrails/components/parameter-label.tsx | Shared parameter label renderer (required marker + info tooltip). |
| packages/apollo-react/src/canvas/components/Guardrails/components/mixed-scopes-banner.tsx | Adds banner for mixed-scope guardrails with “save as new” hint. |
| packages/apollo-react/src/canvas/components/Guardrails/components/mixed-scopes-banner.test.tsx | Tests mixed-scopes banner rendering + a11y. |
| packages/apollo-react/src/canvas/components/Guardrails/components/map-enum-field.tsx | Adds map-enum editor bound to sibling enum-list selection via useWatch. |
| packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-banner.tsx | Adds status banners for disabled/unauthorized/feature-disabled definitions. |
| packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-banner.test.tsx | Tests status banner roles and a11y. |
| packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-scope-selector.tsx | Adds scope/tool targeting selector using chips and self-healing behavior. |
| packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-scope-selector.test.tsx | Tests selector behavior, targeting semantics, and a11y. |
| packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-chip.tsx | Adds chip toggle component (CVA variants) for scopes/options. |
| packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-chip.test.tsx | Tests chip pressed state, interactions, and a11y. |
| packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-action-section.tsx | Adds action configuration section (log/block/filter/escalate). |
| packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-action-section.test.tsx | Tests action section branching + a11y. |
| packages/apollo-react/src/canvas/components/Guardrails/components/field-shell.tsx | Adds non-input “field shell” container with error border option. |
| packages/apollo-react/src/canvas/components/Guardrails/components/field-shell.test.tsx | Tests field shell invalid styling toggle. |
| packages/apollo-react/src/canvas/components/Guardrails/components/enum-list-chips-field.tsx | Adds chip-based enum-list editor for small option sets. |
| packages/apollo-react/src/canvas/components/Guardrails/builder-utils.ts | Adds builder helpers for defaults and required-field validation. |
| packages/apollo-react/src/canvas/components/Guardrails/builder-utils.test.ts | Tests builder utils behaviors and edge cases. |
| packages/apollo-react/src/canvas/components/Guardrails/builder-types.ts | Adds public structural types for persisted guardrail values and builder slots. |
| packages/apollo-react/src/canvas/components/Guardrails/fixtures/host-copy-baselines.ts | Adds transcribed Flow/Agents English baselines for copy parity tests. |
| packages/apollo-react/src/canvas/components/Guardrails/fixtures/definitions-wire.fixtures.ts | Adds realistic wire fixtures for parsing/enrichment/copy tests. |
| packages/apollo-react/package.json | Exposes ./canvas/guardrails subpath and adds deps (class-variance-authority, jest-axe, @types/jest-axe). |
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 81/82 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const [rowIds, setRowIds] = useState<string[]>(() => items.map(() => crypto.randomUUID())); | ||
| const [prevLength, setPrevLength] = useState(items.length); | ||
| if (prevLength !== items.length) { | ||
| setPrevLength(items.length); | ||
| setRowIds((prev) => | ||
| prev.length < items.length | ||
| ? [ | ||
| ...prev, | ||
| ...Array.from({ length: items.length - prev.length }, () => crypto.randomUUID()), | ||
| ] | ||
| : prev.slice(0, items.length) | ||
| ); | ||
| } |
📊 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 |
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 15, 2026, 04:18:40 AM |
2ff0c35 to
c9d8f20
Compare
c9d8f20 to
d658731
Compare
…AL-574] Review of #1139 against the #1107/#1138 threads (plan/review-2026-09-11 §3.2). - Type the fetch mocks as `vi.fn<typeof fetch>`: `vi.fn(async () => ...)` infers a zero-parameter mock, so every `mock.calls[i]?.[1]` assertion was a TS2493/TS2339 under the repo's strict config. CI cannot see it (tests are excluded from `tsc` and biome does not typecheck), so it is checked with a throwaway tsconfig. - `loading` starts `true` when the hook is about to fetch, so a host rendering `loading ? <Spinner/> : <Empty/>` no longer flashes the empty state on first paint. - `refetch` is a no-op while the hook is disabled. It used to issue a real request whose result `parsed` then discarded in favour of `options.definitions`. - JSDoc and README: `options.definitions` is compared by identity (pass a stable reference), a failed request keeps the previous results, and the zod boundary is pinned by a source-level check plus two tests, not by shipped runtime assertions. - Name the map-enum `0..1` step `0.1` default as a product assumption and pin what keeps it safe: after the rebase onto `d658731b`, `min`/`max` are enforced through `validation` in `onChange` mode, but both that path and `getOutOfRangeParameterIds` are number-only, so a synthesized map-enum bound cannot reject a threshold map whose real range is different (harmful content is 0..6). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
64e1ff4 to
6c9b54a
Compare
Integration branch only: it exists so a host can pin one preview package carrying every open apollo stream. Not for merging into main. Rebuilt on 2026-09-11 after the whole stack moved onto #1138's current head (`d658731b`) and picked up a first pass of review fixes on #1139 and #1140. Reset to `feat/apollo-react-guardrail-list` and re-merged `feat/apollo-react-guardrail-palette` (#1147, AL-576). The chip files both branches carry merged clean, being byte-identical again. Of the 17 conflicts, `i18n.ts`, `i18n.test.ts` and the 13 locale catalogs are unchanged on both sides since the previous merge (`8856e4e0`), so its resolution was reused verbatim. `index.ts` is the union of both barrels, biome-sorted, and checked for a lost export. The README was rebuilt from `8856e4e0`'s merged copy with the three deltas since then reapplied (the new base's, #1139's review fixes, #1140's review fixes), all cleanly, so the sections still run along the data flow: definitions layer, list, palette, builder. Verified after the merge: Guardrails suite 453 passing (26 files), tsc and biome clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…AL-574] Review of #1139 against the #1107/#1138 threads (plan/review-2026-09-11 §3.2). - Type the fetch mocks as `vi.fn<typeof fetch>`: `vi.fn(async () => ...)` infers a zero-parameter mock, so every `mock.calls[i]?.[1]` assertion was a TS2493/TS2339 under the repo's strict config. CI cannot see it (tests are excluded from `tsc` and biome does not typecheck), so it is checked with a throwaway tsconfig. - `loading` starts `true` when the hook is about to fetch, so a host rendering `loading ? <Spinner/> : <Empty/>` no longer flashes the empty state on first paint. - `refetch` is a no-op while the hook is disabled. It used to issue a real request whose result `parsed` then discarded in favour of `options.definitions`. - JSDoc and README: `options.definitions` is compared by identity (pass a stable reference), a failed request keeps the previous results, and the zod boundary is pinned by a source-level check plus two tests, not by shipped runtime assertions. - Name the map-enum `0..1` step `0.1` default as a product assumption and pin what keeps it safe: after the rebase onto `d658731b`, `min`/`max` are enforced through `validation` in `onChange` mode, but both that path and `getOutOfRangeParameterIds` are number-only, so a synthesized map-enum bound cannot reject a threshold map whose real range is different (harmful content is 0..6). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
6c9b54a to
74a4be4
Compare
📦 Dev Packages
|
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved review findings include a critical stale-data risk and multiple parser, validation, synchronization, and accessibility defects.
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 (9)
packages/apollo-react/src/canvas/components/Guardrails/components/map-enum-field.tsx:73
- This custom numeric input is the control that renders map-enum validation errors, but it never receives
aria-invalid. Whenerroris present, the visibleFormFieldErroris not accompanied by the invalid state on the input, so assistive technology can miss which control is affected. Pass the error state through asaria-invalid.
<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/map-enum-field.tsx:58
getRequiredEmptyParameterIdsmarks a required map-enum with no keys as invalid, and the builder passes that message througherror, but this early return removes the wholeFormFieldand itsFormFieldError. Clicking Save can therefore block a required empty map-enum with no visible error. Keep a label/error shell when the source has no keys, or suppress validation for this dependent field until its source has a selection.
if (keys.length === 0) return null;
packages/apollo-react/src/canvas/components/Guardrails/components/parameter-label.tsx:25
InfoTooltiprenders a real<button>, so putting it insideLabelnests a labelable control inside a label. Clicking the tooltip can activate the associated input and exposes ambiguous label semantics to assistive technology. Render the tooltip as a sibling of the label, asFormFieldLabeldoes.
{paramDef.tooltip && (
<InfoTooltip content={paramDef.tooltip} aria-label={labels.moreInformation} />
)}
packages/apollo-react/src/canvas/components/Guardrails/definitions-enrich.ts:145
copyis a normal object, so an otherwise-unrecognised wire validator such astoStringresolves to an inheritedObject.prototypemember instead ofundefined. With any parameter,toParameterDefinitionthen readscurated.paramLabels[param.id]from that function and enrichment throws, even though the parser accepted the definition. Check that the validator is an own key before using it as curated copy so unknown wire values remain renderable.
const curated = isByo ? undefined : copy[wire.validator];
packages/apollo-react/src/canvas/components/Guardrails/definitions-parse.ts:170
- The parser's contract says malformed entries never escape as exceptions, but
readValidator(entry)is called after thesafeParsetry/catch. A hostile object or proxy with a throwingvalidatorgetter will therefore throw while constructing the parse issue instead of being reported ininvalid. Make this helper catch property-access failures (returningundefined) before it is used for an invalid entry.
packages/apollo-react/src/canvas/components/Guardrails/guardrail-builder.tsx:415 - The host-owned
saveDisabledgate is applied to the primary button at line 428, but not to the secondarySave as newaction here. WhensaveDisabledis true (for example while a host slot is resolving),Save as newremains enabled andhandleSaveAsNewcan still invokeonSaveAsNew, bypassing the documented host save gate. IncludehostSaveDisabledin this action'sdisabledcondition.
packages/apollo-react/src/canvas/components/Guardrails/use-metadata-form-bridge.ts:72 Object.isonly provides reference equality, so a controlled host that echoes an equivalent cloned array or map treats it as changed on every render. The effect then callssetValuerepeatedly despite the hook's deep-equality contract, causing unnecessary React Hook Form updates and potentially disturbing focused list/map editors. Compare structured values before callingsetValue(or normalize them to stable references).
packages/apollo-react/src/canvas/components/Guardrails/use-metadata-form-bridge.ts:74- The controlled-value sync only iterates keys present in
values; it never clears a field that the host removes fromparameters. After a host drops an optional parameter (for example when normalizing empty values), the old array/map/text value remains in React Hook Form and is still shown, so the UI no longer reflects the controlled source and a later edit can re-submit stale data. Reconcile removed fields to their current defaults/empty state as well as syncing present keys.
packages/apollo-wind/src/components/forms/validation-converter.ts:59 - Because this condition skips the refinement whenever
minLengthis configured, a required string withminLength: 3accepts' 'after satisfying the length rule. That contradicts the sharedisEmptyFieldValuesemantics described immediately above and lets whitespace-only required values through; the refinement should be applied regardless ofminLength.
- Files reviewed: 93/94 changed files
- Comments generated: 4
- Review effort level: Lite
| useEffect(() => { | ||
| if (!enabled) { | ||
| // Disabled: abort anything in flight and drop previous results, so a host that | ||
| // switches to its own payload never renders stale data or a stuck spinner. | ||
| abortRef.current?.abort(); | ||
| setFetched(EMPTY_RESULT); | ||
| setLoading(false); | ||
| setError(null); | ||
| return undefined; | ||
| } | ||
| load(); | ||
| return () => abortRef.current?.abort(); |
| // `.min(1)` rather than `emptyToUndefined`: an empty string here would make a UiPath | ||
| // validator read as bring-your-own, which changes copy resolution and palette grouping. | ||
| byoValidatorName: z.string().min(1).optional(), |
| const [fetched, setFetched] = useState<GuardrailDefinitionsParseResult>(EMPTY_RESULT); | ||
| // `true` on the first render of an enabled hook: the effect below is about to fetch, and a | ||
| // host that renders `loading ? <Spinner/> : <Empty/>` would otherwise flash the empty state. | ||
| const [loading, setLoading] = useState(enabled); |
…AL-574] Review of #1139 against the #1107/#1138 threads (plan/review-2026-09-11 §3.2). - Type the fetch mocks as `vi.fn<typeof fetch>`: `vi.fn(async () => ...)` infers a zero-parameter mock, so every `mock.calls[i]?.[1]` assertion was a TS2493/TS2339 under the repo's strict config. CI cannot see it (tests are excluded from `tsc` and biome does not typecheck), so it is checked with a throwaway tsconfig. - `loading` starts `true` when the hook is about to fetch, so a host rendering `loading ? <Spinner/> : <Empty/>` no longer flashes the empty state on first paint. - `refetch` is a no-op while the hook is disabled. It used to issue a real request whose result `parsed` then discarded in favour of `options.definitions`. - JSDoc and README: `options.definitions` is compared by identity (pass a stable reference), a failed request keeps the previous results, and the zod boundary is pinned by a source-level check plus two tests, not by shipped runtime assertions. - Name the map-enum `0..1` step `0.1` default as a product assumption and pin what keeps it safe: after the rebase onto `d658731b`, `min`/`max` are enforced through `validation` in `onChange` mode, but both that path and `getOutOfRangeParameterIds` are number-only, so a synthesized map-enum bound cannot reject a threshold map whose real range is different (harmful content is 0..6). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
74a4be4 to
9fa041b
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate findings remain across parsing, hook behavior, form synchronization, accessibility, renderer identity, and validation.
Get a fresh assessment by requesting another Copilot review.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (9)
packages/apollo-react/src/canvas/components/Guardrails/fixtures/definitions-wire.fixtures.ts:52
- This fixture says enrichment supplies a
0..1bound, butdefinitions-enrich.tsintentionally leaves an unbounded map withoutmin/maxand only supplies the editorstephint. The stale comment contradicts the save-time validation contract documented by the new tests and can lead hosts to assume an invented range is enforced; describe the value as unbounded with a step hint instead.
// The backend omits the bounds here; enrichment supplies 0..1 step 0.1.
packages/apollo-react/src/canvas/components/Guardrails/components/escalate-action-fields.tsx:182
- In the built-in static-recipient fallback, this
Labelis a sibling with nohtmlFor, while the fallbackInputbelow has neither anidnor an accessible label. Screen readers therefore cannot associate “Email address”/“Group name” with the required value control (and the searchable fallback has the same gap). Give the built-in inputs stable ids and associate the label, while keeping custom render slots responsible for their own controls.
{/* Recipient value */}
<FormField>
<Label>
{recipientTypeLabels[displayedRecipientType] ?? labels.recipientFallbackLabel}
<RequiredIndicator />
</Label>
packages/apollo-react/src/canvas/components/Guardrails/definitions-parse.ts:95
- The shared seeding contract explicitly handles
defaultValue: nullfor boolean parameters, but this schema rejects such a payload before enrichment. A valid definition with a nullable boolean default is therefore dropped asinvalid, so the nullability support is incomplete; accept the nullable form here and update the hand-written wire mirror to match.
packages/apollo-react/src/canvas/components/Guardrails/definitions-parse.ts:120 - Using
.min(1).optional()here rejects the entire definition when the backend sendsbyoValidatorName: ''; it does not normalize the empty marker to an absent value as the parser documentation claims. That removes an otherwise valid non-BYO validator fromdefinitionsinstead of preserving it. Use the existingemptyToUndefinedschema here and assert that the parsed result still contains the definition.
packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:126 provided === undefinedcannot distinguish an omitted override from an explicitly supplied but not-yet-loaded payload. A normal SWR/React Query call such asuseGuardrailDefinitions(ctx, { definitions: data })hasdata === undefinedon its first render, so this hook starts its own request, violating the documented no-fetch override path and potentially issuing duplicate requests. Track whether thedefinitionskey is present and use that presence flag for bothenabledand the parsed-result branch.
packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:131useState(enabled)is evaluated only on the initial mount. If the hook starts disabled and later receives a fetch context, or if a loaded hook switches to a new request context,loadingis stillfalsefor the render before the effect callssetLoading(true). This violates the documented loading contract and lets hosts flash an empty state or render stale results as settled; derive/reset loading for each new enabled request rather than only initializing it once.
packages/apollo-react/src/canvas/components/Guardrails/use-metadata-form-bridge.ts:90- Controlled external updates use
setValuewithout requesting validation, so an earlier resolver error can remain in RHF after the host replaces an invalid value with a valid one. Since this bridge explicitly runs the form inonChangemode, validate the field when syncing a changed value (or explicitly clear/recompute its error).
packages/apollo-react/src/canvas/components/Guardrails/use-metadata-form-bridge.ts:145 setError(..., { type: 'external' })on a registered field is removed by RHF's resolver when a later validation passes, but this effect only re-applies the message when theerrorsprop changes. If a caller keeps an error prop unchanged (for example, it does not provideonClearError), editing the field makes the host error disappear despite the documented "cleared only when the prop drops" contract. Re-apply host errors after validation/value changes or use an error overlay that the resolver cannot replace.
packages/apollo-react/src/canvas/components/Guardrails/use-metadata-form-bridge.ts:90- The sync loop only visits keys present in
next, so removing a parameter from the controlledparametersarray leaves its previous value registered in RHF and still visible. This violates the controlled contract for external updates; track the previous field keys and clear/reset fields that disappear (using the schema/default value) rather than treating omission as no-op.
- Files reviewed: 99/100 changed files
- Comments generated: 6
- Review effort level: Lite
| <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" | ||
| /> |
| 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>; |
| function readValidator(entry: unknown): string | undefined { | ||
| if (typeof entry !== 'object' || entry === null) return undefined; | ||
| const validator = (entry as { validator?: unknown }).validator; | ||
| return typeof validator === 'string' && validator !== '' ? validator : undefined; | ||
| } |
| const getApolloMessageRenderers = (locale: SupportedLocale) => [ | ||
| { | ||
| name: DEFAULT_MESSAGE_RENDERER, | ||
| component: AutopilotChatMarkdownRenderer, | ||
| }, |
| // Bounds only when the backend states them. This layer used to invent 0..1 for the | ||
| // threshold maps that arrive unbounded, which was safe while the bound was an editor | ||
| // hint nothing enforced. #1138's `getOutOfRangeParameterIds` now range-checks | ||
| // `map-enum` rows and hosts gate Save on it, so an invented bound would reject a | ||
| // threshold on a scale the backend never stated. It is enforcement, so the numbers |
…AL-574] Review of #1139 against the #1107/#1138 threads (plan/review-2026-09-11 §3.2). - Type the fetch mocks as `vi.fn<typeof fetch>`: `vi.fn(async () => ...)` infers a zero-parameter mock, so every `mock.calls[i]?.[1]` assertion was a TS2493/TS2339 under the repo's strict config. CI cannot see it (tests are excluded from `tsc` and biome does not typecheck), so it is checked with a throwaway tsconfig. - `loading` starts `true` when the hook is about to fetch, so a host rendering `loading ? <Spinner/> : <Empty/>` no longer flashes the empty state on first paint. - `refetch` is a no-op while the hook is disabled. It used to issue a real request whose result `parsed` then discarded in favour of `options.definitions`. - JSDoc and README: `options.definitions` is compared by identity (pass a stable reference), a failed request keeps the previous results, and the zod boundary is pinned by a source-level check plus two tests, not by shipped runtime assertions. - Name the map-enum `0..1` step `0.1` default as a product assumption and pin what keeps it safe: after the rebase onto `d658731b`, `min`/`max` are enforced through `validation` in `onChange` mode, but both that path and `getOutOfRangeParameterIds` are number-only, so a synthesized map-enum bound cannot reject a threshold map whose real range is different (harmful content is 0..6). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
9fa041b to
5384a89
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate issues remain in parsing, enrichment, hook state transitions, and map validation, plus one fixture documentation nit.
Get a fresh assessment by requesting another Copilot review.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (7)
packages/apollo-react/src/canvas/components/Guardrails/definitions-enrich.ts:67
- This fallback does not actually humanize acronym or snake_case identifiers:
IPAddressbecomesI p address, andsome_paramremainsSome_param. An otherwise valid new parameter with one of these common wire naming styles will therefore show a malformed label. Split acronym boundaries and_/-separators while lowercasing only words after the first, and add regression cases for these forms.
packages/apollo-react/src/canvas/components/Guardrails/definitions-enrich.ts:56 - The parser normalizes an empty
byoValidatorName, but this exported enrichment path acceptsGuardrailDefinitionWiredirectly and treatsbyoValidatorName: ''as BYO because it only checks forundefined. A host usingenrichGuardrailDefinitionswithout the parser would then bypasshiddenValidatorsand lose curated copy for a built-in definition. Keep the BYO predicate consistent with the parser by rejecting the empty string here as well.
packages/apollo-react/src/canvas/components/Guardrails/definitions-parse.ts:120 - The parser currently rejects
byoValidatorName: ''with this.min(1)schema, so the entire definition is added toinvalidinstead of being treated as a managed validator. That contradicts the normalization contract and the parser test; normalize the empty marker toundefinedbefore validation rather than dropping the definition.
packages/apollo-react/src/canvas/components/Guardrails/definitions-parse.ts:97 - The wire contract documented by this layer permits an unset boolean default, and
seedGuardrailParametersalready coerces a null boolean tofalse, butz.boolean()rejects that payload here. A backend definition withtype: 'boolean'anddefaultValue: nullwill therefore be dropped as invalid; make the hand-written wire mirror and parser schema nullable in sync.
packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:131 - Because
useState(enabled)is evaluated only on mount,loadingis stale for one render wheneverenabledor the request key changes. For example, switching fromnullto a context returnsloading: falsebefore the effect callsload, exposing the empty/stale state, while switching tooptions.definitionscan returnloading: trueeven though the supplied payload is already available. This causes the same transition flash that the initial-state fix was intended to prevent; derive the returned loading state from the current request generation or reset it synchronously when the key changes.
packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:188 - When
requestchanges, this effect starts the new fetch but leavesfetcheduntouched.parsedtherefore continues exposing the previous request's definitions whileloadingis true, so switching tenant/account/base URL can render and allow editing old-context guardrails under the new context. Keep fetched data keyed torequestKey(while retaining the same-key refetch behavior), or clear it synchronously when the request key changes.
packages/apollo-react/src/canvas/components/Guardrails/utils.ts:145 - The map editor deliberately keeps values for deselected keys until
syncMapEnumParametersruns at save time, but this check scans every entry in the unsynchronized map.GuardrailBuildercalls it before that save-time reconciliation, so a user can enter an out-of-range value, deselect that key, and still be blocked by an error for a row that is no longer rendered. Validate only the keys currently selected by the map'skeySource(or reconcile before checking).
- Files reviewed: 98/99 changed files
- Comments generated: 1
- Review effort level: Lite
| options: PII_ENTITY_OPTIONS, | ||
| }, | ||
| { | ||
| // The backend omits the bounds here; enrichment supplies 0..1 step 0.1. |
The Guardrails UI, shared by Flow and Agents, as an MUI-free family under
`src/canvas/components/Guardrails`, exported through the narrow
`@uipath/apollo-react/canvas/guardrails` subpath. Members: `GuardrailBuilder`
(the whole Add/Edit screen), `GuardrailFormLayout` (the screen shell), and
`GuardrailValidatorForm` (the validator parameter section).
Built on apollo-wind's forms/ MetadataForm stack rather than its own renderer:
five of the seven parameter types map onto first-class field types, while the
chip-style enum-list, `map-enum` and host `renderParameter` overrides register
as custom components. Strings localize through lingui (`guardrails.*` ids,
14 catalogs).
MetadataForm owns its own state, so this family's controlled contract is
translated onto its plugin seam in exactly one named place,
`useMetadataFormBridge`: it registers the custom components from the first
paint, pushes host values in structurally compared (so an echo of the form's
own emission performs no write and focus survives), pushes host errors in as
`type: 'external'`, and suppresses its own echo while writing. A sync arriving
before `onFormInit` is replayed rather than dropped.
Validation is shared, and the split is deliberate: the schema declares
`required`/`min`/`max` from the definitions with messages from the label
catalog, so they translate; the host owns domain rules and the save-time gate
through `getRequiredEmptyParameterIds` / `getOutOfRangeParameterIds`. Where the
two disagree the host's verdict is what renders — a `text-list` of
whitespace-only rows passes the array's `.min(1)` but counts as empty for the
host predicate — and a test pins that. Custom fields declare a `valueType` so
those constraints bind to them too; `map-enum` has no counterpart shape, so its
required check stays the host's alone.
Save-time correctness: the builder validates the *reconciled* parameters, since
the editor deliberately keeps rows for deselected map keys while the save path
prunes them — reading the raw values let a required map look filled by a stale
key and then persist as `{}`. Host parameter errors drop out of the display and
the Save gate once the user edits past them, so a backend verdict no longer
outlives the edit that fixed it.
`onSaveAsNew` hands the host the original selector verbatim, scopes included.
Narrowing a duplicated guardrail's coverage is a product decision with safety
consequences, so it is a documented contract with the transformation left to
the host rather than an undocumented accident.
Single-line inputs the builder renders itself (name, escalation recipient/app
fallbacks) sit outside the nested MetadataForm's guarded div, so the root
swallows Enter for them too: mounted `inline` inside a host form, Enter would
otherwise trigger the host's implicit submission and skip `handleSave`.
Host errors are re-asserted after the form revalidates: the bridge checks the
field's real error state rather than the last message it wrote, and a values
subscription restores anything the resolver replaces, so the prop stays the
source of truth.
Control ids are namespaced per instance with `useId`, so two inline builders can
share a document without `htmlFor` binding a label to the wrong one, and a
required map-enum surfaces its label and error even when the source selection is
empty and there are no rows to show.
Review round from BenGSchulz: malformed persisted values no longer crash the
builder or slip past the Save gate (one `coerceParameterValueToType` applied to
defaults and wire data alike); `overrideParameterIds` lets a host declare which
parameters it overrides instead of the form probing `renderParameter` on every
render; the chip groups carry a real accessible name; the escalation slots agree
on one error-ownership rule; FieldShell and the chips track apollo-wind's
`future:` layer; and the modal stories no longer open over the docs page.
Malformed wire data is normalised against the definitions — value *and*
discriminator — while host sidecars with no definition pass through untouched,
and a non-finite persisted map threshold falls back rather than reaching onSave.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ilDefinitions Adds the definitions layer the guardrails family was missing: the seam between the `/api/execution/guardrails/definitions` payload and the `GuardrailDefinition`s `GuardrailBuilder` renders. Flow and Agents each carry their own copy of this today, and the two have drifted. - `definitions-wire.ts` mirrors the payload as hand-written types, reusing `GuardrailScope` and `GuardrailDefinitionStatus` from `builder-types` so the wire and display layers cannot diverge. It admits both products' nullability variants. - `definitions-parse.ts` validates unknown input and never throws: a non-array payload sets `inputError`, a bad definition is dropped whole and reported in `invalid`, unknown keys are stripped. zod is private to this module, pinned to the public mirror by a bidirectional assignability check on the hot path, a runtime key-set assertion and a source-level guard, so the folder's emitted declarations carry no schema types. - `definitions-copy.ts` holds the canonical copy for the six built-in validators as 63 lingui messages in the shared canvas catalog, keyed by raw wire values. Translations harvested from both products, 62 of 63 in each of the 12 locales. - `definitions-enrich.ts` resolves that copy onto the wire shape. Pure and React-free; `EnrichedGuardrailDefinition extends GuardrailDefinition`, so its output feeds the builder with no mapping. - `useGuardrailDefinitions` composes the three over `useState` + `fetch` + `AbortController`, following `useDiscoveryModels`. `options.definitions` skips the request entirely, which is how each product keeps its own transport. Unlike `useDiscoveryModels` the context is compared by content, not identity: keying the effect off identity made an inline context object refetch on every render without terminating. Where the products' English differed, all 17 choices are declared with a reason in `definitions-parity.test.ts` and asserted against both products' transcribed copy, so the shared table cannot quietly drift from the tables it replaces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…AL-574] Review of #1139 against the #1107/#1138 threads (plan/review-2026-09-11 §3.2). - Type the fetch mocks as `vi.fn<typeof fetch>`: `vi.fn(async () => ...)` infers a zero-parameter mock, so every `mock.calls[i]?.[1]` assertion was a TS2493/TS2339 under the repo's strict config. CI cannot see it (tests are excluded from `tsc` and biome does not typecheck), so it is checked with a throwaway tsconfig. - `loading` starts `true` when the hook is about to fetch, so a host rendering `loading ? <Spinner/> : <Empty/>` no longer flashes the empty state on first paint. - `refetch` is a no-op while the hook is disabled. It used to issue a real request whose result `parsed` then discarded in favour of `options.definitions`. - JSDoc and README: `options.definitions` is compared by identity (pass a stable reference), a failed request keeps the previous results, and the zod boundary is pinned by a source-level check plus two tests, not by shipped runtime assertions. - Name the map-enum `0..1` step `0.1` default as a product assumption and pin what keeps it safe: after the rebase onto `d658731b`, `min`/`max` are enforced through `validation` in `onChange` mode, but both that path and `getOutOfRangeParameterIds` are number-only, so a synthesized map-enum bound cannot reject a threshold map whose real range is different (harmful content is 0..6). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… enforces [AL-574] `getOutOfRangeParameterIds` used to look at `number` parameters only, so the 0..1 step 0.1 this layer synthesized for an unbounded threshold map was an editor hint nothing could reject a value against. The guardrails family has since widened that check to `map-enum` and hosts gate Save on it, which turns a bound nobody stated into a blocked save on a scale the backend never published. Pass the wire's `min`/`max` through when it sends them and leave them off when it does not, so only real constraints reach the check. `step` stays: neither the check nor `buildFieldValidation` reads it, and without it a 0..1 score steps by 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Promote the read-only status chip and the chip geometry constant it reuses. Copied verbatim from the AL-575 list branch (#1140), which introduced them: the palette needs the same chip for an unauthorized definition, and both PRs branch off the definitions layer rather than stacking, so each carries the shared files and whichever merges second drops its duplicate on rebase. Keep the two copies byte-identical.
… with #1140 wind's `Badge` renders a `<div>`, and the palette entry puts these chips inside its `<button>`, where flow content is invalid. Compose from wind's exported `badgeVariants` on a `<span>` instead: same classes, an element that may live there, `ComponentPropsWithoutRef<'span'>` and `HTMLSpanElement` on the ref. The chip is byte-identical on #1140 and #1147, so this commit lands on both.
…[AL-574] Every guardrails component's i18n test re-implements the same three checks over its own id prefix: English parity, orphaned ids, and translation coverage. The catalog is hand-authored and harvested by a one-off script, so these scans stand in for `lingui extract` and a translation pipeline, and each new component copy-pasted them. They move to `__fixtures__/catalog-coverage` as reporting functions, so a failure still points at the calling test's own line. The definitions layer is the first caller and gains what it was missing: the orphan sweep now covers all thirteen catalogs rather than English alone, and a coverage check that pins the one harvested-English-only entity label instead of leaving the gap invisible. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…AL-574] The centralized section (#1161) needs the green both products already give the BYO origin chip, and had forked its own copy of the chip to get it. The tone belongs where the chip lives, so every consumer gets the same four. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5384a89 to
d233de3
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Moderate parser, hook, humanization, fixture, and localization findings remain unresolved.
Get a fresh assessment by requesting 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:134
- The catalog scan now iterates
CANVAS_LOCALES, which containsen,ru, and 12 translated locales (14 files), not thirteen. This README claim is stale and understates the catalogs covered by the orphan sweep; update the count to match the implementation.
> `src/canvas` uses no lingui macros, so `lingui extract` does not feed this catalog: its
> entries are hand-authored. A test asserts every message reaches `src/canvas/locales/en.json`
> with the same English, and that the catalog carries no `guardrails.definitions.*` id the
> source no longer declares. That test is what extraction would otherwise be doing for you.
packages/apollo-react/src/canvas/components/Guardrails/fixtures/definitions-wire.fixtures.ts:52
- This fixture comment still describes the removed behavior: enrichment no longer supplies
0..1bounds for this unbounded map, as the assertions below now verify. Please update the comment to say that only the0.1step hint is supplied and the min/max remain unset, otherwise the fixture documents the old save-blocking behavior.
// The backend omits the bounds here; enrichment supplies 0..1 step 0.1.
packages/apollo-react/src/canvas/components/Guardrails/definitions-enrich.ts:66
- The fallback humanizer splits every uppercase character, so a valid uncurated parameter id such as
APIKeybecomesA p i key(andIPAddressbecomesI p address). This path is used for backend parameters not yet in the curated table, so preserve consecutive-uppercase runs when inserting word boundaries.
packages/apollo-react/src/canvas/components/Guardrails/definitions-parse.ts:192 - This contract comment is narrower than the implementation: the schemas also normalize empty
byoConnectorName,byoConfigurationId,folderPath, andfolderKey, andparametersdefaults to[]. That makes the documented wire behavior inaccurate for callers relying on the parser boundary; document these normalizations (or remove the claim that there are only two).
packages/apollo-react/src/canvas/components/Guardrails/definitions-parse.ts:170 - The advertised “never throws” guarantee is still breakable for an object with a throwing
validatorgetter (or proxy):safeParsecan enter thecatch, butreadValidator(entry)then reads the same property outside any guard and throws while constructing the issue. Read the diagnostic validator defensively as well, so malformed entries are always reported rather than escaping the parser.
packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:131 useState(enabled)only applies on mount. If a host starts disabled (for example while it suppliesoptions.definitionsor has a null context) and then switches to a self-fetching context,enabledis true butloadingis still false for the render before the effect callsload(). The documentedloading ? ... : <Empty/>consumer can flash an empty state on that transition (and the same happens when changing request keys after a settled fetch). Track the request that has been loaded or derive a synchronous “new enabled request” state soloadingis true before the effect runs.
packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:117- The hook claims the request is compared by context content, but
JSON.stringifymakes header insertion order part of that identity. Rebuilding equivalent headers as{ Authorization, Tenant }and then{ Tenant, Authorization }aborts and reissues the definitions request even though the HTTP headers are unchanged. Canonicalize the header entries (or otherwise compare them order-independently) before building this key.
packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:224 - When a render switches from an active request to
options.definitionsor a null context,enabledbecomes false immediately but these fields still return the previous request'sloading/erroruntil the passive effect runs. An in-flight request can therefore make a host show a spinner or transport error alongside the already-selected host payload for a paint; return disabled-state values synchronously (for example, mask them withenabled).
packages/apollo-react/src/canvas/locales/de.json:268 - This translation changes the Microsoft product name from Azure AI Content Safety to
Azure AI Content Security, which is a different/nonexistent service name and can mislead users about the validator dependency. Keep the product nameAzure AI Content Safetyin this localized description.
packages/apollo-react/src/canvas/locales/es.json:260 - The Spanish tooltip leaves
integerin English (Valor integer entre...), so this newly added localization is grammatically mixed and does not match the rest of the sentence. Use the Spanish termenterohere.
- Files reviewed: 72/73 changed files
- Comments generated: 1
- Review effort level: Lite
| "guardrails.definitions.harmful_content.option.harmfulContentEntities.Sexual": "Violencia", | ||
| "guardrails.definitions.harmful_content.option.harmfulContentEntities.Violence": "sexual", |
Builds the shared guardrail definitions layer AL-574 asks for, in the apollo-react guardrails family. This is the seam between the
/api/execution/guardrails/definitionspayload and theGuardrailDefinitionsGuardrailBuilderalready renders. Flow and Agents each carry their own copy of this today, and the two have drifted.Base, and what to review
Base
main, 8 commits,dev-packageslabelled. The first two are #1107 (a96ef79f) and #1138(
9fa10765), each now a single squashed commit; review only the last six:feat(apollo-react): shared guardrail definitions layer and useGuardrailDefinitions- the work.fix(apollo-react): review fixes for the guardrail definitions layer [AL-574]- a first pass ofreview fixes, listed under Review fixes below.
fix(apollo-react): stop inventing map-enum bounds the range check now enforces [AL-574]- seeSynthesized map-enum bounds below.
feat(apollo-react): guardrail status chip, shared with #1140andfix(apollo-react): render the guardrail status chip as a span, shared with #1140- moved downhere from feat(apollo-react): guardrail list section [AL-575] #1140 and feat(apollo-react): add-guardrail palette [AL-576] #1147, see Shared leaves move here below.
test(apollo-react): share the canvas catalog scans across the family [AL-574]- same section.Retargeted from
feat/apollo-react-guardrails-familytomainon 2026-09-11:pr-checks,dev-publishandpreview-deployare all gated onbranches: [main, 'support/**'], so against afeature base they never ran and the absent checks meant "not run", not "passed".
For most of that time they still did not run, because every branch in this stack conflicted with
maininpackages/apollo-wind/src/components/ui/select.tsxand GitHub skips thepull_requestworkflows when it cannot build a test merge. That is resolved upstream as of 2026-09-14: this
branch and the three leaves all merge into
mainwith zero conflicts, so checks and the-pr1139preview pair should publish normally from here.
Rebased 2026-09-14 onto #1138's current head (
9fa10765), which had itself moved onto #1107'scurrent head (
a96ef79f). Both were squashed to one commit apiece and carry +556/-92 over 32files of new content versus the head this PR previously sat on, so the 18 commits that used to be
below this work are gone. Two consequences are visible here: the form now validates number
min/maxthrough zod inonChangemode rather than as DOM attributes, andgetOutOfRangeParameterIdshas been widened tomap-enum(see Synthesized map-enum bounds).Merge order is still #1107, then #1138, then this.
Built fresh from #1107/#1138, the rescoped Jira ticket and Confluence §7.3/§7.4.5. Nothing is ported
from the closed #1132.
What lands
definitions-wire.tsGuardrailDefinitionWire,GuardrailParameterDefinitionWiredefinitions-parse.tsparseGuardrailDefinitionsdefinitions-copy.tsGUARDRAIL_COPY_EN,GUARDRAIL_COPY_EN_MESSAGES,useGuardrailDefinitionCopydefinitions-enrich.tsenrichGuardrailDefinitions,isByoGuardrailDefinition,humanizeGuardrailParameterId,withGuardrailFolderMetadatause-guardrail-definitions.tsuseGuardrailDefinitionsPlus a README section, exports from
Guardrails/index.ts(nopackage.jsonchange,./canvas/guardrailsalready points there), and the newguardrails.definitions.*ids in the canvas catalog.Contract highlights
inputError; a single bad definition is dropped whole and reported ininvalid, which is what both products already do entry by entry. Unknown keys stripped. Transport errors and data errors are separate channels: a malformed payload leaveserrornull.definitions-parse.tsand pinned to the hand-written mirror by a bidirectional assignability check that sits on the hot path intoWireDefinition(so it cannot be dropped as dead code), plus two tests: a key-set assertion over both shapes, and a source-level guard that reads the folder's files and fails on a zod import outside the parser.grep zod dist/canvas/components/Guardrails/**/*.d.tsis empty.EnrichedGuardrailDefinition extends GuardrailDefinition, so its output feeds the builder unmapped.options.definitionsskips the request entirely, which is how Agents keeps SWR, Flow studio and workbench keep react-query, and the vsix keeps postMessage.hiddenValidatorshides nothing by default and never hides a BYO definition. Which validators a product exposes is an entitlement decision, so it stays with the caller.One deliberate divergence from
useDiscoveryModelsThe context is compared by content, not identity. Keying the effect off context identity means an inline context object refetches on every render, and since every response sets state the loop never terminates. The hook test caught it at 17,640 calls before the fix.
useDiscoveryModelsstill has this footgun; worth a separate look.Synthesized map-enum bounds, and why they are gone
Enrichment used to give an unbounded
map-enumparameter0..1step0.1: some backends omit thebounds on the threshold maps, and an unbounded numeric editor for a confidence score is a data-entry
hazard. That default was safe only while nothing enforced it, and the JSDoc said so in as many
words: "Before widening either to map-enum, replace this default with something the backend
states."
#1138 has now widened it.
getOutOfRangeParameterIdscoversmap-enumas of9fa10765, and hostsgate Save on it, so a bound this layer invented would block a save over a number the backend never
published. Enrichment now passes the wire's
min/maxthrough when they are sent and leaves themoff when they are not.
stepstays a hint, since neither that check norbuildFieldValidationreads it and without it a
0..1score steps by 1.Nothing on today's wire changes behaviour: PII's
entityThresholdsarrives unbounded and defaultsto
0.8, harmful content arrives with its own0..6. What changes is that a future unbounded mapon another scale cannot be Save-gated against a number we made up. The test that pinned the old
"never checked" premise now pins the new one, including that a map on the backend's own bounds
does report.
Shared leaves move here
Two things the leaf PRs were each carrying their own copy of now land once, on the layer they all
branch from. The leaves inherit them and their own diffs shrink by the same amount.
GuardrailStatusChip(components/guardrail-status-chip.tsxplus its test), cherry-pickedverbatim from feat(apollo-react): add-guardrail palette [AL-576] #1147 where it was already an isolated commit. It was byte-identical on feat(apollo-react): guardrail list section [AL-575] #1140 and
feat(apollo-react): add-guardrail palette [AL-576] #1147 and had to be kept that way by hand, one cherry-pick per edit. A
<span>composed fromwind's exported
badgeVariantsrather than theBadgecomponent, which renders a<div>: thepalette entry puts these chips inside its
<button>, where flow content is invalid. Comes withGUARDRAIL_CHIP_GEOMETRY, extracted fromguardrail-chip.tsxso the interactive and read-onlychips stay one system. Nothing in this PR renders it yet; AL-578 and the two leaves do.
__fixtures__/catalog-coverage.ts). Every component'si18n.test.tswasre-implementing the same three checks over its own id prefix, about 40 lines apiece: English
parity, orphaned ids, and translation coverage. They are reporting functions rather than
assertions, so a failure still points at the calling test's own line. The definitions layer is the
first caller and gains what it was missing: the orphan sweep now covers all thirteen catalogs
instead of English alone, and there is a coverage check that pins
FIPassportNumberby namerather than leaving the gap invisible.
Canonical copy moves onto lingui
The display copy for the six built-in validators currently lives twice, in Agents'
OOB_GUARDRAILS_I8Nand Flow'sbuildValidatorDisplayInfo. Here it is 63 lingui messages in the shared canvas catalog, so both products get the same wording and the same translations, and the strings enter the real loc pipeline instead of a host-side constant.Message ids use raw wire values (
USSocialSecurityNumber), never a transcribed slug. Transcribing is exactly how the two products ended up keying the same Finland entity asfinNationalIdandfiNationalId.Translations harvested from whichever product each string was adopted from: 62 of 63 ids in each of the 12 locales. The gap is
FIPassportNumber, which Agents has not had translated; it falls back to English per key.ruis empty, matching both products and this package's existing convention.QA-visible copy changes
The two products' English differs in 17 places. Each choice is declared with a reason in
definitions-parity.test.tsand asserted against both products' transcribed copy, so the suite fails on an undeclared difference, a stale declaration, or a third wording we invented.Agents users will see: shorter validator descriptions (the "This validator is designed to..." preamble is gone from four of them);
harmfulContentEntitiesreads "Content categories" and its thresholds "Severity thresholds";ipEntitiesreads "Content types"; PII thresholds pluralized; LLM-as-judgethresholdreads "Strictness";SelfHarmreads "Self-harm"; a new LLM-as-judge cost note.Flow users will see: three new parameter tooltips (PII, prompt-injection and harmful-content thresholds) and the Finland passport entity, which Flow renders as a raw value today.
prompt_injectionkeeps Agents' wording as the deliberate exception to the concision rule, because the Noma Security attribution is load-bearing.Review fixes
The second commit, from a pass over this PR against the #1107/#1138 review threads:
vi.fn<typeof fetch>).vi.fn(async () => ...)infers a zero-parametermock, so every
mock.calls[i]?.[1]assertion in the hook suite was aTS2493/TS2339under therepo's strict config: 10 errors CI cannot see, since tests are excluded from
tscand biome doesnot typecheck. Same class as the two Copilot catches on feat(apollo-wind): string-list field, tooltip metadata, and forms repairs #1107. Checked with a throwaway tsconfig
that drops the test/story excludes; this PR's files are clean under it.
loadingstartstruewhen the hook is about to fetch. It startedfalse, so the firstrender of a self-fetching host was
{ definitions: [], loading: false }and aloading ? <Spinner/> : <Empty/>host flashed the empty state.refetchis a no-op while the hook is disabled. It used to issue a real request whose resultwas then discarded in favour of
options.definitions.options.definitionsis compared by identity, notcontent (pass a stable reference; an SWR or react-query result already is), a failed request keeps
the previous results, and the zod boundary is a source-level check plus two tests.
0..1map-enum assumption and pinned it with a test. The third commit then removedthe assumption outright once feat(apollo-react): guardrails component family under canvas #1138 started enforcing it, per the section above.
Verification
Re-run in full after the 2026-09-14 rebase.
tsc --noEmit: clean.all pre-existing in feat(apollo-react): guardrails component family under canvas #1138's own suites (
guardrail-builder.test.tsx7,form-schema-builder.test.ts2, plus one each inguardrail-builder.stories.tsx,guardrail-form-layout.test.tsxandguardrail-validator-form.test.tsx). CI typechecks neithertests nor stories, so they are invisible to it. Nothing here touches those files.
biome check: clean.Guardrailsdirectory: 333 passing, 1 failing.The 9 failures are 8 + 1, and none of them are this PR's:
localStoragefailures incanvas/utils/Storage.test.tsandcanvas/hooks/useStorageState.test.ts: Node 24 ships an experimentallocalStorageglobal thatis
undefinedwithout--localstorage-fileand shadows happy-dom's.guardrail-validator-form.test.tsx > localization > resolves the resolver validation messages from the catalog, not wind Englishrenders the English"Must be at most 1" instead of the Japanese. It is not caused by this branch. Checked by
swapping feat(apollo-react): guardrails component family under canvas #1138's own
src/canvas/locales/*back in underneath this branch, where it failsidentically, and re-run after a clean
apollo-windbuild. Worth a look upstream before feat(apollo-react): guardrails component family under canvas #1138merges.
Review questions
withGuardrailFolderMetadata, because resolving it needs each product's connections API (Agents pagesfetchResources, Flow callsgetConnectionById). Do you want it inside the hook instead, as aresolveConnectionscallback?