feat(apollo-react): guardrail action and escalation section [AL-579] - #1172
andreizdrali-uipath wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate issues remain unresolved: fallback actionApp errors are not rendered, and the escalate path lacks the section root hook.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Promotes guardrail action and escalation UI to standalone public APIs for custom-rules builders.
Changes:
- Adds public action/escalation components, types, labels, exports, layouts, and slots.
- Updates supporting form infrastructure, localization, tests, stories, and documentation.
- Applies the app-picker alert spacing fix.
- Review follow-ups: Two moderate findings (2 votes each): render
actionApperrors when no picker is provided, and preserve the section root hook on the escalate path.
File summaries
| File | Review note |
|---|---|
pnpm-lock.yaml |
Reviewed; no final comment. |
packages/apollo-wind/src/index.ts |
Reviewed; no final comment. |
packages/apollo-wind/src/components/ui/textarea.tsx |
Reviewed; no final comment. |
packages/apollo-wind/src/components/ui/select.tsx |
Reviewed; no final comment. |
packages/apollo-wind/src/components/ui/info-tooltip.tsx |
Reviewed; no final comment. |
packages/apollo-wind/src/components/ui/info-tooltip.test.tsx |
Reviewed; no final comment. |
packages/apollo-wind/src/components/ui/info-tooltip.stories.tsx |
Reviewed; no final comment. |
packages/apollo-wind/src/components/ui/index.ts |
Reviewed; no final comment. |
packages/apollo-wind/src/components/ui/form-field.tsx |
Reviewed; no final comment. |
packages/apollo-wind/src/components/ui/form-field.test.tsx |
Reviewed; no final comment. |
packages/apollo-wind/src/components/ui/datetime-picker.tsx |
Reviewed; no final comment. |
packages/apollo-wind/src/components/forms/validation-converter.test.ts |
Reviewed; no final comment. |
packages/apollo-wind/src/components/forms/string-list-field.tsx |
Reviewed; no final comment. |
packages/apollo-wind/src/components/forms/schema-serializer.ts |
Reviewed; no final comment. |
packages/apollo-wind/src/components/forms/schema-serializer.test.ts |
Reviewed; no final comment. |
packages/apollo-wind/src/components/forms/rules-engine.ts |
Reviewed; no final comment. |
packages/apollo-wind/src/components/forms/README.md |
Reviewed; no final comment. |
packages/apollo-wind/src/components/forms/metadata-form.stories.tsx |
Reviewed; no final comment. |
packages/apollo-wind/src/components/forms/index.ts |
Reviewed; no final comment. |
packages/apollo-wind/src/components/forms/form-schema.ts |
Reviewed; no final comment. |
packages/apollo-wind/src/components/forms/form-designer.tsx |
Reviewed; no final comment. |
packages/apollo-wind/src/components/forms/demo-mocks.ts |
Reviewed; no final comment. |
packages/apollo-wind/src/components/forms/custom-controls.stories.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/test/setup.ts |
Reviewed; no final comment. |
packages/apollo-react/src/i18n/index.ts |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/locales/ru.json |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/index.ts |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/utils.ts |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/use-metadata-form-bridge.ts |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/types.ts |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/render-parameter-bridge.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/index.ts |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/guardrail-validator-form.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.test.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.stories.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/definitions-wire.ts |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/definitions-parse.ts |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/definitions-parity.test.ts |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/definitions-copy.test.ts |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/parameter-label.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/mixed-scopes-banner.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/mixed-scopes-banner.test.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/map-enum-field.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-chip.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-chip.test.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-banner.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-banner.test.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-scope-selector.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-scope-selector.test.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-chip.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-chip.test.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-action-section.tsx |
Moderate (2 votes): restore the section root hook on the escalate path. |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-action-section.test.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/field-shell.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/field-shell.test.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/components/enum-list-chips-field.tsx |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/builder-utils.ts |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/builder-types.ts |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/__fixtures__/host-copy-baselines.ts |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/__fixtures__/definitions-wire.fixtures.ts |
Reviewed; no final comment. |
packages/apollo-react/src/canvas/components/Guardrails/__fixtures__/catalog-coverage.ts |
Reviewed; no final comment. |
packages/apollo-react/package.json |
Reviewed; no final comment. |
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 99/100 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.
| if (action.$actionType === 'escalate') { | ||
| return ( | ||
| <EscalateActionFields | ||
| action={action} | ||
| onChange={onActionChange} | ||
| actionTypeSelect={actionTypeSelect} |
84ef308 to
c07d3a9
Compare
|
Apollo Coded App preview deployments are ready.
|
Dependency License Review
License distribution
Excluded packages
|
📦 Dev Packages
|
There was a problem hiding this comment.
🔵 Needs a closer look
The two unresolved review issues must be addressed before approval.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
packages/apollo-react/src/canvas/components/Guardrails/components/escalate-action-fields.tsx:209
- The recipient-value fallbacks still render a visible
LabelwithouthtmlFor, while theInputs below have noid; because they are siblings, the standalone escalation fields give the recipient control no accessible name even though their validation message is associated. Add a generated label/control association for both fallback paths, and provide an equivalent accessible-label seam for replacement slots.
{/* One rule across all three slots, matching `renderAppPicker`: a slot receives `error`
and owns rendering it, so the form renders no message of its own for a claimed field.
The sibling `FormFieldError` used to sit outside this ternary, so a host doing the
obvious `<Input error={ctx.error} />` got the message twice. Every fallback goes
through `Input`'s `error` prop, which renders the message *and* wires
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-action-section.tsx:119
- This slot is only rendered for non-escalation actions: the
escalatebranch above returnsEscalateActionFields, whose root isdata-slot="guardrail-escalate-fields". Consumers using the newly added section slot to style or locateGuardrailActionSectionwill therefore fail whenever the action isescalate; keep the section slot on the root for both branches.
<div data-slot="guardrail-action-section" className={cn('@container', className)}>
- Files reviewed: 73/74 changed files
- Comments generated: 0 new
- Review effort level: Lite
📊 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 18, 2026, 03:08:27 AM |
…ilDefinitions [AL-574] Turns the `GET /api/execution/guardrails/definitions` payload into 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 by hand, admitting both products' nullability variants, and reuses `GuardrailScope`/`GuardrailDefinitionStatus` so wire and display cannot drift. - `definitions-parse.ts` validates with zod and never throws: a non-array sets `inputError`, one bad definition is dropped whole into `invalid`. Blank display strings are the single normalization, since one would beat curated copy and render an empty label; blank identifiers fail the entry instead of silently changing a definition's identity. zod stays private, pinned to the hand-written mirror by a bidirectional assignability check on the hot path plus a key-set test and a source-level import guard, so no schema type reaches the emitted `.d.ts`. - `definitions-copy.ts` carries the six built-in validators' display copy as 63 lingui messages in the shared canvas catalog, replacing Agents' `OOB_GUARDRAILS_I8N` and Flow's `buildValidatorDisplayInfo`. Ids use raw wire values, never a transcribed slug, which is how the two products ended up keying the same entity as `finNationalId` and `fiNationalId`. English only: the l10n sync owns the other catalogs, as it does for every other string here. - `definitions-enrich.ts` resolves copy onto validated wire definitions. Pure and React-free, so Flow's vsix bridge calls it directly. Curated wins at definition level, wire wins at parameter level, BYO takes no curated copy. - `use-guardrail-definitions.ts` composes the three. `options.definitions` skips the request entirely, so each product keeps its own transport. The context is compared by content rather than identity, unlike `useDiscoveryModels`, where an inline object refetches every render and never settles. Results carry the request key that produced them, so a tenant switch cannot keep serving the previous tenant's guardrails. The 17 places the two products' English differs are each declared with a reason in `definitions-parity.test.ts` and asserted against both products' transcribed copy, so the suite fails on an undeclared difference or a wording we invented. Also lands two things the leaf PRs were each carrying their own copy of, since #1140, #1147 and #1161 all branch from here. `GuardrailStatusChip` is a read-only pill for a guardrail row: deliberately not `GuardrailChip`, which wraps a Radix `Toggle` and would put fake buttons in the tab order, and a `<span>` composed from wind's `badgeVariants` rather than `Badge`, which renders a `<div>` that is invalid inside the palette entry's `<button>`. `GUARDRAIL_CHIP_GEOMETRY` is extracted so the interactive and read-only pills stay one system. Its tones are harvested, not invented: `success` is the green both products give a BYO origin or connector chip, `info` the blue both give a "Preview" lifecycle label (Flow `bg-blue-100 text-blue-700`, Agents `semantic.colorInfoBackground`), so neither product's colour changes when the leaves adopt it. The label truncates with the full text on the chip's `title`, because the pill is a fixed height and the text is the host's: a governance label or a connector name long enough to wrap rendered two lines and spilled out of its own background. The label sits in an inner span so it can truncate at all, since `text-overflow` does not reach the anonymous flex item bare text becomes inside `inline-flex`. `__fixtures__/catalog-coverage.ts` holds the catalog scans that stand in for `lingui extract`, which never sees `src/canvas` because it uses no macros. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
c07d3a9 to
1eea16e
Compare
1eea16e to
19b7570
Compare
…n [AL-579] `GuardrailActionSection` and `EscalateActionFields` have shipped inside `GuardrailBuilder` since the family landed, but only the builder could reach them. Both hosts need them on their own for the custom-rules builder, where the action is edited outside the OOTB screen. The components are unchanged for the builder; what this adds is the standalone surface: - `labels` is optional and partial. `GuardrailActionLabels` is a `Pick` over the builder's own keys and resolves the same `guardrails.builder.*` ids, so there are no new strings to translate and neither path can word one differently. - `GuardrailActionErrors` / `GuardrailEscalateActionErrors` replace the inline error literals, and `GuardrailEscalateAction` names the escalate arm of `GuardrailAction` for hosts holding one in state. - `EscalateActionFields` takes `actionTypeSelect` as optional and gains `asGridItems`, so a host can lay the three cells out in its own grid or stack them. With `actionTypeSelect` given, nothing changes. - `className` on both. The recipient field's `<label>` now points at the input it names, and slots get `ctx.labelId` to name their own control; previously neither fallback input had an accessible name. The app-picker note gets `mt-0`, since `AlertDescription`'s top offset assumes a title above it. Contract otherwise unchanged: the escalation target stays slot-driven, filter stays host territory through `showFilter` / `filterContent`, and no flags, telemetry, product types or zod cross the boundary. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
19b7570 to
e53e58f
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Three moderate findings remain in the definitions enrichment and loading paths.
Review details
Suppressed comments (3)
packages/apollo-react/src/canvas/components/Guardrails/definitions-enrich.ts:116
param.idis wire-controlled, but these plain-object lookups also read inherited keys. A new parameter namedtoString,constructor, or__proto__on a curated validator therefore gets a function/object as its label or tooltip instead of the humanized fallback (and can break the form). Use own-property lookups for parameter labels, tooltips, and option-label groups, just as the validator lookup above does.
label:
param.displayName ?? curated?.paramLabels[param.id] ?? humanizeGuardrailParameterId(param.id),
packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:206
- Disabling through
options.definitionsdoes not changerequestKey, so the stamped result remains current even though this branch only clearsinFlight. When the override is later removed, the hook can immediately expose the old request's definitions withloading: falseuntil the effect starts the new fetch; clear the settled result in this disabled path.
// Only the in-flight flag needs clearing; the results are already unreachable, since
// their stamp cannot match a disabled hook's request.
abortRef.current?.abort();
setInFlight(false);
packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:137
- When a fetch for the current request fails,
settled.erroris retained. If a caller then supplies the documentedoptions.definitionsoverride,requestKeyandisCurrentare unchanged, so this returns the old transport error alongside the valid supplied data. The error should be hidden whenever fetching is disabled.
const error = isCurrent ? settled.error : null;
- Files reviewed: 28/28 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
Three moderate hook findings remain unresolved; the README contract qualification is also outstanding.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
packages/apollo-react/src/canvas/components/Guardrails/README.md:76
- This contract is broader than the implementation: a failed request only retains previous results when it is a refetch of the same request key. When the context changes, the hook deliberately clears the old tenant's results before reporting the new request's error. Please document that qualification here so hosts do not rely on stale data remaining visible across tenant/context switches.
- **A failed request keeps the previous results.** `error` is set and `definitions` still hold
the last good payload, so a transient 503 on a `refetch` does not empty a list the user is
looking at. Render on `error` first if you want it to replace the data. Disabling the hook
does clear the fetched state.
packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:206
- The disabled branch only makes the old stamp unreachable while
requestKeyis''; it does not clearsettled. If the host disables the hook and later enables the same context again,isCurrentbecomes true immediately, so the old tenant's definitions and a stale error are shown withloading: falsewhile the new request is in flight. Clear the settled state when disabling, as the README contract promises.
// Only the in-flight flag needs clearing; the results are already unreachable, since
// their stamp cannot match a disabled hook's request.
abortRef.current?.abort();
setInFlight(false);
packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:124
definitions?: unknowncan be explicitly supplied asundefinedwhile a host-owned SWR/react-query request is still loading. Because this checks the value rather than whether the property was supplied, the hook treats that state as transport-enabled, starts a second fetch, and can expose fetched data instead of the host result, contrary to the documented "when it is present no request is made" contract. Track property presence (and use the same flag when selectingparsed) so an explicitly suppliedundefinedremains host-controlled.
// A host-provided payload replaces the request entirely rather than racing it.
const enabled = provided === undefined && request !== null;
- Files reviewed: 28/28 changed files
- Comments generated: 1
- Review effort level: Lite
|
|
||
| const isCurrent = settled.key === requestKey; | ||
| const fetched = isCurrent ? settled.result : EMPTY_RESULT; | ||
| const error = isCurrent ? settled.error : null; |
AL-579, epic AL-526. Review the last commit; the one before it is PR #1139, which this branches off.
What
GuardrailActionSectionandEscalateActionFieldsship insideGuardrailBuilderbut were never exported, so the only way to edit a guardrail action was the whole OOTB screen. The custom-rules builder edits the action on its own in both products (Agents'GuardrailActionBuilder+GuardrailEscalationAssetRecipient, Flow'sGuardrailActionSection+EscalateActionBuilder), so both are exported here with the props a standalone caller needs.GuardrailActionSection/GuardrailActionSectionPropsEscalateActionFields/EscalateActionFieldsPropsGuardrailActionErrors,GuardrailEscalateActionErrorsGuardrailBuilderErrorsGuardrailEscalateActionGuardrailActionGuardrailActionLabels,GuardrailActionLabelKey,GUARDRAIL_ACTION_EN_LABELS,GUARDRAIL_ACTION_LABEL_KEYS,resolveGuardrailActionLabels,useGuardrailActionLabelsStandalone the section needs
actionandonActionChange; everything else is optional, so nothing changes for the builder:labelsis optional and partial.GuardrailActionLabelsis aPickover the builder's own keys and resolves the sameguardrails.builder.*ids, so there are no new strings and neither path can word one differently. The builder keeps passing its full labels object.EscalateActionFieldstakesactionTypeSelectas optional and gainsasGridItems, so a host can lay the three cells out in its own grid or stack them. Flow's own component has both modes. WithactionTypeSelectgiven, which is what the section passes, the markup is unchanged.GuardrailActionErrors/GuardrailEscalateActionErrors, andGuardrailEscalateActionnames the escalate arm for a host holding one in state.classNameon both.Contract unchanged: the escalation target stays slot-driven (
renderRecipientSearch,renderStaticRecipient,renderAppPicker,escalateHelp), filter stays host territory throughshowFilter/filterContent, callbacks are intents, and no flags, telemetry, product types or zod cross the boundary.Two fixes in passing, both on the escalation fields:
<label>now points at the input it names, and a slot getsctx.labelIdto name its own control. Neither fallback input had an accessible name (#1138 thread), and it matters more here because the fallbacks are what a standalone caller gets.mt-0on the app-picker note, whoseAlertDescriptionoffset assumes a title above it. Same one-line workaround as the family's other title-less alerts, pending thealertVariantsfix.Plus a README section, stories under
Components/UiPath/Guardrail Action Sectionwith mock slot renderers for the escalate variants, and both suites extended for the standalone defaults.Open questions
Props both hosts' action builders have and this does not. Flagged rather than guessed at.
disabled. Agents disables the Assign-to select when group assignment is off; Flow disables its whole editor while deliberately keeping the drag handle live. A plaindisabled?: booleanwith the flag staying host-side, a narrowerrecipientTypeDisabled?, or leave it to a hostfieldset?role="group" aria-labelledby. This ships content only. Host chrome, as with the centralized section'shideHeader, or an opt-inheaderpair here?escalateHelpis static. On an app error Agents swaps the marketplace line for a different one, so a host has to condition the node on the sameerrors.actionAppit passes in. Enough, or should the slot see the error state?errorprop of its own. Flow'sDirectorySearchInputhas none, so its slot returns control +FormFieldErroras a fragment, which lands outside theFormFieldanatomy (raised on flow-workbench#4025). The README now states the rule; actx.errorNodewould make it structural instead.Smaller, same reasoning: Flow gates error display with a
showErrorsboolean where a host here withholdserrorsinstead; Agents lays the escalate row out at 20/20/25/rest against this 2-column grid; and six copy divergences reach Agents only, inherited from the family's wording ("Blocking reason" for "Block reason", "Assign to" for "Assign app to", "Action App" for "Action app", "Email address" / "Group name" for their capitalized twins, and the recipient label taking the type's name where Agents always says "Recipient"). Flow's wording is unchanged throughout.Verification
tsc -p tsconfig.jsonclean, and a strict pass over the folder's tests and stories reports nothing in this commit's files.biome lint/formatclean on them,rslib buildclean, no zod in the emitted.d.ts, no lockfile change.dev-packagescomment below and is named afterrefs/pull/1172/merge, not the head sha. The host PRs that pinned6.45.2-pr1172.2f2cc67(flow-workbench#4025, Agents#6278) need a re-pin, sincedev-publishunpublishes the superseded build.🤖 Generated with Claude Code