Skip to content

feat(apollo-react): add-guardrail palette [AL-576] - #1147

Merged
andreizdrali-uipath merged 1 commit into
mainfrom
feat/apollo-react-guardrail-palette
Sep 22, 2026
Merged

andreizdrali-uipath merged 1 commit into
mainfrom
feat/apollo-react-guardrail-palette

Conversation

@andreizdrali-uipath

@andreizdrali-uipath andreizdrali-uipath commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

AL-576, the add-guardrail palette: the picker both products open from their guardrails list.
Third member of the shared family in
packages/apollo-react/src/canvas/components/Guardrails/.

One commit over #1139, which is one commit over main. Review the last commit only.

The Jira description is stale. It still says apollo-wind and component-scoped locales, both
superseded. Read it as apollo-react family, strings as lingui ids in the shared canvas catalog.

What ships

File Exports
guardrail-palette.tsx GuardrailPalette, GuardrailPaletteProps
guardrail-palette-utils.ts groupGuardrailsForPalette, getGuardrailPaletteItemId
palette-types.ts GuardrailPaletteDefinition, GuardrailPaletteGroup
components/guardrail-palette-item.tsx GuardrailPaletteItem (not in index.ts: the narrow surface is the palette)
i18n.ts (extended) GuardrailPaletteLabels, GUARDRAIL_PALETTE_EN_LABELS, GUARDRAIL_PALETTE_EN_MESSAGES, resolveGuardrailPaletteLabels, useGuardrailPaletteLabels

Grouped definitions, an optional create-custom entry, and the loading, failed and empty states.
GuardrailStatusChip and GUARDRAIL_CHIP_GEOMETRY are on #1139, where all three leaves inherit
them from one place.

Contract

The host filters, the palette offers. Definitions arrive pre-filtered (flags, entitlements,
FeatureDisabled / Disabled, scope) and both callbacks are intents: the builder that opens
next, the default name it starts with and the telemetry stay with the product. No flag, product
type or telemetry crosses the boundary.

Design decisions

  • A list of buttons, not a Select or a Command. It composes into a dialog, an inline
    overlay or a whole sidebar, and shows descriptions without a popover. Flow's SelectContent
    already fights the primitive to get there (multi-line items, disabled items, sticky headers,
    h-auto min-h-9 trigger, two !important viewport overrides). cmdk is a combobox modelling a
    selection, while palette entries are one-shot actions, and its navigation skips
    aria-disabled items by construction, which this palette needs to keep reachable.
    QA-visible for Flow: the type dropdown becomes a one-step list.
  • One tab stop, roving focus inside it. Tab enters and leaves the palette instead of walking
    a user through a create-custom entry, six validators and every BYO group. Arrow keys step
    across group boundaries, Home and End jump to the ends, movement clamps rather than wraps.
    The focused entry comes off the keyboard event target, not document.activeElement, which
    retargets to the shadow host inside Agents' shadow root; a test renders into an open shadow
    root and arrows down.
  • Unauthorised is offered, chipped and aria-disabled, never disabled, so a keyboard
    user reaches the chip that says why it cannot be picked. Flow's behaviour. QA-visible for
    Agents
    , where the entry currently opens a builder that then refuses to save.
  • Definitions are generic over the eight fields the palette reads.
    EnrichedGuardrailDefinition and Agents' OutOfTheBoxGuardrailDefinition both satisfy it, and
    onSelectOotb hands the host's own object back with parameters intact, so there is nothing
    to look up again.
  • Only the picker ships. Flow opens a 500px dialog and an inline overlay that renders the
    chosen builder underneath; Agents takes over the sidebar with a back button and its own create
    affordance. Composing either is a handful of lines (InAHostSidebar is 24, InADialog 37), so
    a wrapper modelling both would be a worse contract than none.
  • onCreateCustom is opt-in by presence. Flow offers custom guardrails for scope === 'Tool'
    only and passes the prop conditionally; Agents keeps the affordance in its own header, omits
    the prop and gets no duplicate entry.
  • An empty catalog produces no groups, unlike both products' groupGuardrailsForPalette,
    which returns one empty group. That is what makes the empty state reachable: Flow guards its
    own empty message on groups.length === 0, which its implementation can never return.
  • Entry identity is validator, or byoValidatorName:byoGuardrailConnectionId for BYO. Flow
    keys BYO entries on the name alone and collides when two connections expose one validator.
  • Chip tones are harvested. The BYO connector chip is success, the green both products
    already give it; the "Preview" chip is info, the blue both give that one.
  • Groups name themselves with aria-labelledby pointing at the visible header rather than
    repeating it in aria-label, and the loading line is an <output> for its implicit
    role="status".
  • Both sorts are localeCompare() with no locale argument, matching both products today.
    Recorded in the JSDoc rather than changed, because changing it is a product decision.

Strings

Nine guardrails.palette.* ids in one builder function, the definitions-copy.ts idiom. All
nine are harvested from the products' own catalogs, so the English is what they already ship:
eight from Flow's addGuardrailPalette_* keys, list-aria-label from Agents'
guardrails.palette.list_aria_label.

English only, like the rest of the package: chore(l10n): sync from Localization owns the other
thirteen catalogs. i18n.test.ts asserts English parity with the source and that no catalog
keeps a guardrails.palette.* id the source has dropped.

One item for the loc pass: the two products translate the word itself differently in German,
"Leitplanke" in Flow against "Leitlinien" in Agents.

Also in this commit

Wind's AlertDescription is unconditionally mt-1, which assumes an AlertTitle above it,
while alertVariants pins the icon at [&>svg]:top-3.5. With no title the text lands 4px below
the icon, visible in the Load Failed story. mt-0 at the call site, the same hunk as #1140
and #1161 so the branches merge without a conflict. The real fix is one line in wind's
alertVariants ([&>[data-slot=alert-description]:first-child]:mt-0), which would let all three
workarounds go; separate PR.

Verification

Guardrails suite 427 passing (23 files), jest-axe on both rendering suites. tsc --noEmit,
biome lint and format clean. Stories and tests typechecked separately with a throwaway tsconfig,
since the package tsconfig excludes both and CI therefore does not cover them: no errors in this
PR's files.

Open questions

  1. The Unauthorized chip is amber (tone="warning"). Flow renders it neutral
    (bg-muted text-muted-foreground) and Agents does not render it at all. Neutral matches the
    one product that ships it; amber carries more meaning. Reviewer's call.
  2. Should FeatureDisabled / Disabled definitions also render as disabled entries? They do
    not today: both products filter them out before rendering, so Unauthorised is the only
    non-Available status that reaches a correctly filtered palette. Say the word and the
    status-to-chip mapping generalizes.

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings September 10, 2026 11:13
@andreizdrali-uipath andreizdrali-uipath added the dev-packages Adds dev package publishing on pushes to this PR label Sep 10, 2026
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Apollo Coded App preview deployments are ready.

Project Status Preview Updated (PT)
apollo-design Ready Preview · Logs Sep 22, 2026, 01:58:59 AM
apollo-docs Ready Preview · Logs Sep 22, 2026, 01:58:59 AM
apollo-landing Ready Preview · Logs Sep 22, 2026, 01:58:59 AM
apollo-vertex Ready Preview · Logs Sep 22, 2026, 01:58:59 AM

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Dependency License Review

  • 1937 package(s) scanned
  • ✅ No license issues found
  • ⚠️ 2 package(s) excluded (see details below)
License distribution
License Packages
MIT 1708
ISC 88
Apache-2.0 55
BSD-3-Clause 27
BSD-2-Clause 23
BlueOak-1.0.0 8
MPL-2.0 4
MIT-0 3
CC0-1.0 3
MIT OR Apache-2.0 2
(MIT OR Apache-2.0) 2
Unlicense 2
LGPL-3.0-or-later 1
Python-2.0 1
CC-BY-4.0 1
(MPL-2.0 OR Apache-2.0) 1
Unknown 1
Artistic-2.0 1
(WTFPL OR MIT) 1
(BSD-2-Clause OR MIT OR Apache-2.0) 1
CC-BY-3.0 1
0BSD 1
(MIT OR CC0-1.0) 1
MIT AND ISC 1
Excluded packages
Package Version License Reason
@img/sharp-libvips-linux-x64 1.3.3 LGPL-3.0-or-later LGPL pre-built binary, not linked
khroma 2.1.0 Unknown MIT per GitHub repo, missing license field in package.json

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

📦 Dev Packages

🧹 Dev packages cleaned up after PR close.

Last updated: 2026-09-22 02:07:29 PT

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical and moderate findings affect selection safety, key identity, error-state rendering, and accessibility.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds a reusable Apollo React guardrail palette for built-in and BYO guardrails, with localization, grouping utilities, status chips, stories, and tests.

Changes:

  • Adds generic palette rendering, types, callbacks, and grouping/identity helpers.
  • Adds localized palette labels and catalog coverage.
  • Adds shared status-chip UI and supporting fixtures/tests.
File summaries
File Review summary
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/index.ts Reviewed; no final comment.
packages/apollo-wind/src/components/forms/validation-converter.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/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/field-renderer.tsx Moderate (1 vote): associate the MultiSelect label and control for an accessible field name.
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/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-guardrail-definitions.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/palette-types.ts Reviewed; no final comment.
packages/apollo-react/src/canvas/components/Guardrails/index.ts Reviewed; no final comment.
packages/apollo-react/src/canvas/components/Guardrails/i18n.test.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-palette.tsx Moderate (1 vote each): suppress the empty state when a load error exists and distinguish rendered group keys to avoid collisions.
packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette.test.tsx Reviewed; no final comment.
packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette.stories.tsx Reviewed; no final comment.
packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette-utils.ts Moderate (2 votes each): make item IDs collision-free and disambiguate synthetic and user-provided group keys.
packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette-utils.test.ts 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/form-schema-builder.ts 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-palette-item.tsx Critical (1 vote): prevent spread props from overriding the guarded onClick for unauthorized entries.
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-palette-item.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 Reviewed; no final comment.
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__/guardrail-palette.fixtures.ts Reviewed; no final comment.
packages/apollo-react/src/canvas/components/Guardrails/__fixtures__/definitions-wire.fixtures.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

Suppressed comments (3)

packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette.tsx:86

  • When a load fails before any definitions are available, error is truthy but isEmpty is still true, so the palette renders both “Failed to load built-in validators” and “No guardrails available.” The latter incorrectly presents a transport failure as a valid empty catalog; suppress the empty state when an error is present while preserving stale definitions under the banner.
    packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette.tsx:118
  • group.key is used directly as the React key, but named BYO groups take their key from user/connection data while __uipath__ is reserved for the synthetic UiPath group. A valid folder or connector named __uipath__ therefore produces duplicate React keys, which can cause unstable reconciliation when the catalog changes. Namespace the rendered key by group kind and distinguish the unnamed BYO group instead of using group.key alone.
    packages/apollo-wind/src/components/forms/field-renderer.tsx:474
  • MultiSelect deliberately uses its id to allow a <label> to name the trigger, but this renderer omits both sides of that association. As a result, enum-list fields rendered through this path (including guardrail lists with more than eight options) are announced as the generic “Select options...”/selection count instead of their field label. Pass the field name to both the label and the control, and cover the accessible name in the renderer test.
  • Files reviewed: 93/94 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

📊 Coverage + size by package

Per-package coverage and bundle size on this PR. New-line coverage = of the source lines this PR adds or changes, the % hit by tests.

Package Coverage New-line coverage Packed (gzip) Unpacked vs main
@uipath/apollo-core 42.01 MB 50.16 MB ±0
@uipath/apollo-react 45.9% 78.1% (89/114) 7.72 MB 29.91 MB +7.6 KB
@uipath/apollo-ui-icons 2.86 MB 6.96 MB ±0
@uipath/apollo-wind 469.4 KB 2.96 MB −4 B
@uipath/ap-chat 85.8% 43.90 MB 56.64 MB −35 B

"Coverage" is each package's own coverage.include scope (e.g. apollo-core instruments only scripts/). "Packed"/"Unpacked" come from npm pack --dry-run and only cover built packages — "—" means not measured this run (package not affected / not built). "vs main" is the packed (gzipped) delta against the last successful main build (the package-sizes artifact from the Release workflow); "—" there means no main baseline was available this run. The baseline is main's latest build, not this PR's exact merge-base, so it includes any drift since the branch diverged. Packages with no vitest config are omitted.

@andreizdrali-uipath

Copy link
Copy Markdown
Contributor Author

Published previews, both green:

PR apollo-react apollo-wind
this PR (#1147) 6.43.0-pr1147.95dda0a 2.48.0-pr1147.95dda0a
preview build (#1148), palette + list 6.43.0-pr1148.f612ad1 2.48.0-pr1148.f612ad1

Host sessions want #1148's pair: this PR's preview has no list, and #1140's has no palette.
Note the minor bump from the #1140 pair (main moved to 6.43.0 / 2.48.0).

CI: Test Coverage, Typecheck, Lint, Format and Build green. The two red Audit * Dependencies
checks are the repo's Dependabot backlog on main, identical on #1138 and #1140; no dependency
is added here and Detect Lockfile Changes is green.

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Storybook visual diff

⚠️ Visual changes detected: 11 changed, 9 added (of 409 compared, 389 unchanged). View report

Baseline is the deployed main Storybook, so changes merged to main after this branch was last updated can also appear here. Logs

Updated (PT): Sep 21, 2026, 04:46:36 AM

Copilot AI review requested due to automatic review settings September 11, 2026 08:34
@andreizdrali-uipath
andreizdrali-uipath force-pushed the feat/apollo-react-guardrail-palette branch from 27643cd to 03bb89c Compare September 11, 2026 08:34
andreizdrali-uipath added a commit that referenced this pull request Sep 11, 2026
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>
Comment thread packages/apollo-wind/src/components/forms/metadata-form.test.tsx Fixed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Four moderate findings remain involving invalid chip markup, key collisions, and an unintended React/i18n dependency.

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 (3)

packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-palette-item.tsx:50

  • GuardrailStatusChip renders the wind Badge, which is a <div>, but these chips are inserted into the <span> at this line and ultimately inside the palette's <button>. That produces invalid <button><span><div>… markup (and React validateDOMNesting warnings) for every definition entry; render the status chip as phrasing content or change the item structure so no div-based badge is nested in the button.
        <span className="flex flex-wrap items-center gap-1.5">
          <span className="text-sm font-medium">{name}</span>
          {chips}

packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette-utils.ts:83

  • These keys are not namespaced by group kind, so a customer-controlled folderPath or connector name of __uipath__ creates the same React key as the trailing UiPath group (and __byo__ collides with the nameless BYO group). The palette can then emit duplicate-key warnings and reconciliation can reuse the wrong group; prefix BYO keys separately from the synthetic UiPath key (and update the helper's key contract/tests).
    packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette-utils.ts:2
  • This helper is documented as pure/React-free, but importing isByoGuardrailDefinition as a runtime value from definitions-enrich pulls in its GUARDRAIL_COPY_EN dependency and therefore definitions-copy.ts, which imports React and useSafeLingui. A caller that only needs grouping now loads the full copy/i18n graph (and cannot use this as a genuinely non-React utility). Move the BYO predicate to a small dependency-free module shared by both files, or otherwise avoid this runtime dependency.
  • Files reviewed: 110/111 changed files
  • Comments generated: 1
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 11, 2026 09:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

One or more issues must be addressed before approval.

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 (4)

packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-palette-item.tsx:60

  • The chips slot is inserted inside the palette item's <button>, but GuardrailStatusChip delegates to wind Badge, which renders a <div>. This produces flow-content <div> elements inside both a button and the surrounding <span>, violating their phrasing-content models and potentially producing inconsistent accessibility trees. Make the status chip render as a phrasing element such as <span>, or change the composition so it is not nested inside those elements.
        <span className="flex flex-wrap items-center gap-1.5">
          <span className="text-sm font-medium">{name}</span>
          {chips}
        </span>

packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-chip.tsx:30

  • Badge renders a <div>, so this status chip becomes a <div> inside the <button> emitted by GuardrailPaletteItem (and inside its <span> chip row). That violates the button/phrasing-content contract and can produce invalid DOM and accessibility behavior for every connector, unauthorized, or preview chip. Use a span-based status-chip implementation or render these chips outside the button; mirror the shared component change in the sibling list PR.
    <Badge
      ref={ref}
      variant={TONE_VARIANT[tone]}
      data-slot="guardrail-status-chip"
      className={cn('h-5', GUARDRAIL_CHIP_GEOMETRY, className)}

packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette-utils.ts:75

  • The reserved synthetic keys are not namespaced from named BYO keys. A valid folderPath or connector name of __uipath__ collides with the trailing UiPath group's key, and __byo__ can collide with the nameless BYO group; React then receives duplicate keys and may reconcile the wrong group when the catalog changes. Namespace all named BYO keys (and update the utility tests), or derive the render key from the group kind rather than the raw folder/connector name.
    packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette.tsx:80
  • activeEntry is an array index, but the entries can be reordered or have the create-custom item inserted/removed while the palette is mounted. In that case the focused button keeps focus while a different button receives tabIndex=0, so re-entering with Tab no longer returns to the previously focused entry, contrary to the documented roving-focus contract. Track the active entry by its stable item id (or reconcile the index against the current entry list) when the catalog changes.
  • Files reviewed: 110/111 changed files
  • Comments generated: 1
  • Review effort level: Lite

andreizdrali-uipath added a commit that referenced this pull request Sep 11, 2026
… 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.
Copilot AI review requested due to automatic review settings September 11, 2026 10:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

One or more issues must be addressed before approval.

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 (4)

Previously missed (1) — in code that hasn't changed since the last review.

packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette-utils.ts:1

  • This utility is documented and exported as React-free, but importing isByoGuardrailDefinition from definitions-enrich pulls in definitions-copy's runtime useMemo/useSafeLingui dependency. A non-React caller of groupGuardrailsForPalette therefore still loads React and can fail when the peer is absent. Keep the BYO predicate in a value-only module or define it locally in this utility.

packages/apollo-react/src/canvas/components/Guardrails/form-schema-builder.ts:86

  • buildFieldValidation is attached to every guardrail field here, but GuardrailValidatorForm runs MetadataForm with schemaMode: 'onChange' and this version has no disableValidation prop. Required/range resolver errors consequently appear while editing even though the documented guardrail contract is validation-free and host errors are the source of truth. Remove these validation rules from this adapter or add and actually pass an explicit validation opt-out.
    packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette-utils.ts:88
  • Folder and connector names are arbitrary strings, but the grouping code reserves __uipath__ and __byo__ as synthetic keys. A BYO folder named __uipath__ collides with the trailing UiPath group (and a __byo__ folder collides with the nameless BYO group), producing duplicate React keys and unstable group reconciliation. Namespace synthetic and user-derived keys so the domains are disjoint.
    packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette.tsx:80
  • The roving position is stored as a numeric index, so it no longer identifies the item when the palette changes. For example, if onCreateCustom is added while the second definition is focused, activeEntry stays 1; the previously focused definition shifts to index 2 and gets tabIndex={-1}, while a different item becomes the tab stop. The same happens when refreshed/localized definitions reorder. Track the active item's stable key (with a fallback when it disappears) and derive its current flat index so the documented return-to-last-focused-item behavior is preserved.
  • Files reviewed: 110/111 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread packages/apollo-wind/src/components/forms/validation-converter.ts Outdated
andreizdrali-uipath added a commit that referenced this pull request Sep 11, 2026
… 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.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Address the three moderate findings involving group-key collisions and stale hook state or errors.

Review details

Suppressed comments (3)

packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette-utils.ts:89

  • The synthetic __uipath__ key can collide with a real BYO folder or connector whose name is __uipath__; likewise, __byo__ can collide with a named group when an unnamed BYO group is present. Because these values are used directly as React keys, such a catalog produces duplicate keys and can cause React to reuse the wrong group's DOM during updates. Namespace generated group keys or otherwise allocate collision-free keys instead of mixing raw user names with reserved literals.
  if (uipathDefinitions.length > 0) {
    groups.push({
      key: '__uipath__',
      header: uipathGroupLabel,
      isByo: false,

packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:206

  • settled is never reset when enabled becomes false. If the panel disables this hook and later enables the same context again, settled.key still matches requestKey, so the old definitions are returned and loading is initially false before the new request runs. This contradicts the documented clear-on-disable behavior and can briefly show stale catalog data; reset settled to NOTHING_SETTLED in this branch.
    packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:137
  • When a request for the current context has failed, settled.error remains populated. If the host then supplies options.definitions for that same context, enabled becomes false but this expression still returns the old error because isCurrent is true, so consumers render a failure banner alongside valid host-provided definitions. Only expose the transport error while the fetch path is enabled (or clear it when entering override mode).
  • Files reviewed: 34/34 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

andreizdrali-uipath added a commit that referenced this pull request Sep 16, 2026
…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>
Copilot AI review requested due to automatic review settings September 16, 2026 11:53
@andreizdrali-uipath
andreizdrali-uipath force-pushed the feat/apollo-react-guardrail-palette branch from d7c96ff to 0c5b1ca Compare September 16, 2026 11:53
andreizdrali-uipath added a commit that referenced this pull request Sep 16, 2026
…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>
@andreizdrali-uipath
andreizdrali-uipath force-pushed the feat/apollo-react-guardrail-palette branch from 0c5b1ca to a8b46eb Compare September 16, 2026 11:59

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Three moderate issues remain in key generation, failed-state rendering, and hook state reset.

Review details

Suppressed comments (3)

packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette-utils.ts:55

  • These keys are used as React keys, but host-controlled folder/connector names are copied directly into the same namespace as the synthetic keys below. A folder or connector named __uipath__ collides with the UiPath group, and a named group __byo__ collides with the nameless BYO group; React will warn about duplicate keys and can reconcile the wrong entries. Generate collision-free namespaced keys (or keep an internal React key separate from the public group key).
    .map(([key, list]) => ({
      // Neither a folder nor a connector name: both products still render it, unheaded.
      key: key ?? '__byo__',
      header: key ?? null,
      isByo: true,
      definitions: [...list].sort(byDisplayName),

packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette.tsx:83

  • When the request fails before any definitions arrive, error is truthy but isEmpty remains true, so the failed state renders both the load-failure banner and No guardrails available. That empty message asserts a successful empty catalog even though the contents are unknown (and the LoadFailed story passes exactly this shape); suppress the empty state while an error is present or render a dedicated failed state.
  const isEmpty = groups.length === 0 && onCreateCustom === undefined;

packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:206

  • Disabling the hook only makes the current result unreachable while requestKey is empty; it does not clear settled. If the same context is enabled again, settled.key === requestKey makes the old tenant's definitions visible before the new request, and a failed re-fetch keeps those stale results because it is treated as a same-key refresh. Reset the settled stamp on the disable path so re-enabling always starts from an empty/loading state as documented.
  • Files reviewed: 34/34 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 16, 2026 12:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Four moderate review issues remain in key generation and definition-hook request/state handling.

Review details

Suppressed comments (4)

packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette-utils.ts:13

  • When byoGuardrailConnectionId is absent, every BYO definition with the same byoValidatorName gets the identical name: key. The public palette type and parser both allow that case, so two such entries produce duplicate React keys and can be reconciled as the same row. Either reject BYO entries without a connection id before rendering or use another stable per-definition discriminator instead of collapsing the missing value to an empty suffix.
  return definition.byoValidatorName === undefined
    ? definition.validator
    : `${definition.byoValidatorName}:${definition.byoGuardrailConnectionId ?? ''}`;

packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette.tsx:187

  • group.key is not guaranteed to be unique: a real BYO folder/connector named __uipath__ collides with the synthetic UiPath key, and a real __byo__ name collides with the nameless BYO group. Because this value is used directly as the React key, those valid inputs produce duplicate sibling keys and can make React reconcile the wrong group after definitions change. Include a collision-free discriminator (for example the group index) in the rendered key, or generate opaque keys in the utility.
                key={group.key}

packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:206

  • This branch only makes the current settled result unreachable while requestKey is ''; it never clears it. If the hook is disabled and then re-enabled for the same context (for example, a feature/tenant panel is toggled), isCurrent becomes true again and the old definitions/error are rendered before and during the refetch, contrary to the documented clear-on-disable behavior and potentially exposing stale data. Reset the settled stamp when disabling.
    packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:115
  • The key is described as content-based, but JSON.stringify preserves the insertion order of headers. A host that reconstructs the same header set in a different order will get a new requestKey, abort the current request, and refetch; alternating order can prevent the hook from settling. Canonicalize the header entries before serializing (or use a stable serializer).
  • Files reviewed: 34/34 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@andreizdrali-uipath
andreizdrali-uipath marked this pull request as ready for review September 16, 2026 12:59
andreizdrali-uipath added a commit that referenced this pull request Sep 16, 2026
…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>
Copilot AI review requested due to automatic review settings September 16, 2026 13:50
@andreizdrali-uipath
andreizdrali-uipath force-pushed the feat/apollo-react-guardrail-palette branch from a8b46eb to 718f73a Compare September 16, 2026 13:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Unresolved moderate findings affect chip geometry, key uniqueness, loading/error states, and hook lifecycle state.

Review details

Suppressed comments (7)

packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-chip.stories.tsx:40

  • The story says this row is demonstrating the definition-status tones, but the palette maps Unauthorised to tone="warning" (the amber chip). Showing Unauthorized as error makes the Storybook reference disagree with the component users will see; use the same warning tone here.
      <GuardrailStatusChip tone="error">Unauthorized</GuardrailStatusChip>

packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-chip.tsx:42

  • badgeVariants already contributes py-0.5, so adding h-5 leaves a 16px text line with 8px of vertical padding plus borders inside a 20px border-box. The status label can overflow its fixed-height pill and is visibly tighter/clipped than intended; reset the inherited vertical padding (or avoid the fixed height) when composing the smaller chip.
        'h-5',

packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette-utils.ts:53

  • The raw folder/connector value is used as a sibling React key alongside reserved synthetic keys. A valid BYO folder named __uipath__ collides with the trailing UiPath group (and __byo__ can collide with the nameless BYO group), causing duplicate-key reconciliation and potentially moving/reusing the wrong entries. Namespace or otherwise escape raw group keys while keeping header as the display value.
  const groups: Array<GuardrailPaletteGroup<T>> = [...byoGroups.entries()]
    .sort(([a], [b]) => (a ?? '').localeCompare(b ?? ''))
    .map(([key, list]) => ({
      // Neither a folder nor a connector name: both products still render it, unheaded.
      key: key ?? '__byo__',
      header: key ?? null,

packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette.tsx:169

  • useGuardrailDefinitions deliberately keeps the last successful definitions while a same-request refetch() is in flight, but this branch replaces the whole item tree with the loading line whenever isLoading is true. During revalidation, users therefore lose all existing choices (contrary to the documented stale-cache behavior); render the loading output alongside the entries whenever ootbDefinitions is non-empty, and suppress entries only for an initial empty load.
      {isLoading ? (
        // `<output>` for its implicit `role="status"`, with no styling of its own.
        <output className="flex items-center gap-2 px-2 py-3 text-sm text-muted-foreground">
          <Loader2 className="size-4 animate-spin" aria-hidden="true" />
          {labels.loading}
        </output>
      ) : (

packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette.tsx:83

  • When a load fails with no stale definitions, this condition still renders the empty-state text alongside the error banner, so the failed story tells users both “Failed to load…” and “No guardrails available.” Keep the empty state mutually exclusive with an error (as the model picker does with !effectiveError) while still allowing stale entries when groups is non-empty.
  const isEmpty = groups.length === 0 && onCreateCustom === undefined;

packages/apollo-react/src/canvas/components/Guardrails/guardrail-palette.tsx:187

  • group.key is also used as the React key, but it shares a namespace with the synthetic __uipath__ key below. A valid BYO folder or connector named __uipath__ therefore produces duplicate keys in groups.map, which can make React reconcile the wrong group when the catalog changes. Prefix the React key by group kind (or otherwise keep synthetic and host-provided keys in separate namespaces) without changing the public group key.
                key={group.key}

packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:207

  • This disabled branch only makes the settled result unreachable while requestKey is empty; it never resets settled. If the hook is disabled and then enabled again for the same request, isCurrent becomes true and the old tenant/catalog is returned immediately before the new fetch settles, despite the documented contract that disabling clears fetched state. Reset the settled stamp here as well as aborting the request.
  • Files reviewed: 34/34 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@andreizdrali-uipath
andreizdrali-uipath changed the base branch from main to feat/apollo-react-guardrail-definitions-layer September 21, 2026 07:33
andreizdrali-uipath added a commit that referenced this pull request Sep 21, 2026
…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>
@andreizdrali-uipath
andreizdrali-uipath force-pushed the feat/apollo-react-guardrail-definitions-layer branch from b4e90c4 to 0cde1e2 Compare September 21, 2026 07:54
@andreizdrali-uipath
andreizdrali-uipath force-pushed the feat/apollo-react-guardrail-palette branch 3 times, most recently from bae31c3 to c52285b Compare September 21, 2026 09:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add `GuardrailPalette`, the picker both products open from their guardrails
list: grouped definitions, an optional create-custom entry, and the loading,
failed and empty states.

The host filters and the palette offers. Definitions arrive pre-filtered
(flags, entitlements, `FeatureDisabled`/`Disabled`, Tool scope), both callbacks
are intents, and no telemetry, flag or product type crosses the boundary.
Definitions are generic over the eight fields the palette reads, so
`EnrichedGuardrailDefinition` and a product's own type both go in and come back
out of `onSelectOotb` unchanged.

Grouping and entry identity are re-derived from what both products already
ship: one unheaded group in payload order without BYO definitions, otherwise a
group per BYO folder or connector plus a trailing UiPath group, and BYO entries
keyed by validator name and connection id. An `Unauthorised` definition is
offered, chipped and not choosable (`aria-disabled`, so it stays reachable),
which is Flow's behaviour; Agents lets it through to a builder that then
refuses to save. Two chips take harvested tones rather than neutral grey: the
bring-your-own connector chip is `success`, the green both products already give
it, and the "Preview" lifecycle chip is `info`, the blue both products give that
one (added to `GuardrailStatusChip` on #1139 for this and the list).

Entries are real buttons on wind's interactive-item idiom, not a `div` with
`role="listitem"` and its own key handler: hover is `bg-accent` gated on the
enabled branch, disabled is `opacity-50`, and the focus ring matches `Button`.
The palette is a single tab stop with roving focus, where Arrow/Home/End cross
group boundaries and `aria-disabled` entries are included so the `Unauthorised`
chip stays reachable. The focused entry is located from the keyboard event
target rather than `document.activeElement`, which retargets to the shadow host
inside Agents' shadow root and would make every arrow key a silent no-op there;
a test renders into an open shadow root and arrows down. Groups name themselves
with `aria-labelledby` pointing at the visible header rather than repeating it.

Only the picker ships. Flow's dialog and inline overlay and Agents' sidebar
takeover are host orchestration, shown in the stories rather than modelled in a
wrapper.

Nine `guardrails.palette.*` lingui ids, harvested from the two products' own
catalogs so the English is what they already ship. English only, like every
other string in this package: `chore(l10n): sync from Localization` owns the
other thirteen catalogs. The i18n test uses the family's shared catalog scans
(`__fixtures__/catalog-coverage`) rather than its own locale list and file
reader. The palette's label resolver reuses the family's `mergeLabels` helper.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The reviewed changes have no unresolved blocking issues.

Review effort: Lite
Findings: None

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev-packages Adds dev package publishing on pushes to this PR pkg:apollo-react size:XXL 1,000+ changed lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants