Skip to content

feat(apollo-react): guardrail list section [AL-575] - #1140

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

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

Conversation

@andreizdrali-uipath

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

Copy link
Copy Markdown
Contributor

AL-575 (epic AL-526): GuardrailList, the applied-guardrails list both products render under a tool or an agent. Rows with their lifecycle and status chips, BYO notices, per-row edit and remove, drag reordering, and the empty state.

Replaces the rendering half of GuardrailsListingSection.tsx in Agents (968 LOC) and GuardrailsEditor.tsx + SortableGuardrailItem.tsx in Flow (860). The orchestration around it (palette, builder, remove dialog, save-as-new, telemetry) stays host-side.

One commit over #1139, whose commit is in this range only until it merges. GuardrailStatusChip lives there; review it on #1139.

Contract

The host filters, the component renders. GuardrailList never drops a row.

  • No feature flag crosses the boundary. Agents' six enableGuardrail* booleans and Flow's canvas.guardrails entitlement plus scope filter stay in the adapters.
  • Callbacks are intents. onRemove reports the request; the confirmation, the scoped-removal unwind and the write stay host-side, because the two products confirm with different copy and unwind tool-scoped guardrails differently.
  • onReorder reports the reordered visible array plus the move ({ from, to, id }), so a host rendering a filtered view (Flow's per-tool view over an agent-level list) can splice it back without the package knowing a fuller list exists.
  • definitions is read for three things only: the provider line, the two BYO notices and the status chip. A BYO row matches on validator name alone, so rebinding a configuration to another connection still resolves. The definition type is the minimum the list reads, which both GuardrailDefinition and AL-574's EnrichedGuardrailDefinition satisfy, so the list compiles and tests green without the definitions layer present.

Where the two products disagree

Concern Agents Flow Here
Row identity name id id ?? name, override with getItemId
Read-only isReadOnly disabled disabled
Reorder while read-only off on follows disabled; Flow passes reorderDisabled={false}
Row actions overflow menu inline buttons inline buttons; renderItemActions swaps in the menu and receives defaultActions
Clickable row yes no no, rowActivatesEdit opts in
Scope/action copy localized raw raw, formatScopes / formatAction localize host-side
Provider + description in a tooltip inline lines inline lines, renderRowTooltip adds the tooltip

Every addition is opt-in, so an adopting host gets a swap that renders what it renders today: statusChips, previewChip, unstyled, hideHeader, footer, emptyState, rowActivatesEdit, renderRowTooltip and the two formatters all default to today's behaviour. Flow passes two props, Agents ten. The README has the table with reasons.

The two BYO notices are not gated: both products already show them, and a guardrail that cannot run is not an opt-in detail.

Decisions worth arguing with

  • The drag transform is CSS.Translate, never CSS.Transform. useSortable derives its layout transform from the row's before and after rects, so with variable row heights (a description line, a BYO notice) CSS.Transform emits a scaleY that visibly squashes or stretches the row's text mid-animation. verticalListSortingStrategy never produces a scale of its own, so dropping it costs nothing. Verified against the installed @dnd-kit/sortable source; the published docs do not mention it.
  • With reorder off, no drag machinery mounts. The DndContext, SortableContext and sensors live in a SortableRows component rendered only when the list is reorderable, because useSensors is a hook and would otherwise run for every non-reorderable list, which is most of them. A test asserts the live region is absent with reorder off.
  • The drag handle is a real button carrying setActivatorNodeRef, so it is keyboard reachable and focus returns to it after a drop. Agents' handle is an aria-hidden icon with the listeners on it. A fix, not a regression.
  • Rows highlight on hover unconditionally, like wind's TableRow: a row is a hover target whether or not clicking it opens the editor. Only with rowActivatesEdit does the body also take cursor-pointer and its own focus ring, since only then is it a control. The tint is accent (--surface-hover), not muted (--surface-overlay), which is the panel the list sits on and would be invisible against it. The family-wide rule is a README section shared with feat(apollo-react): centralized guardrails section and read-only details [AL-578] #1161. Nothing inside the row is pulled past that tint: p-1 gives the row 4px, which is exactly the reach of an icon button's ring-offset-2 focus ring, so the drag handle carries no negative margin and its glyph sits an icon button's padding inside the leading edge rather than optically on it.
  • The chip is not GuardrailChip. That wraps a Radix Toggle, so it is a focusable button with pressed state; these are read-only labels and would put fake buttons in the tab order. The "Preview" chip takes the family's info tone and status chips their own, both harvested from the colours the two products already ship.
  • The local-vs-governance axis is called administration, never "origin". Both products' *OriginChip already means BYO versus UiPath-managed, which is a different axis travelling on byoValidatorName. The status mapping is the pure, exported getGuardrailListChips, so AL-578's centralized section reuses it.
  • GuardrailStatusBanner and MixedScopesBanner become public, as the ticket asks, each with a story. MixedScopesBanner also gains the mt-0 that the other title-less alerts have: wind's AlertDescription is unconditionally mt-1, which assumes an AlertTitle above it, so a title-less alert renders its text 4px below its icon. The same hunk is on feat(apollo-react): add-guardrail palette [AL-576] #1147 and feat(apollo-react): centralized guardrails section and read-only details [AL-578] #1161 so the three merge clean; the real fix is one line in wind's alertVariants.

Strings

17 guardrails.list.* ids through useGuardrailListLabels. 12 take their English from Flow's canvas catalog, asserted equal to Flow's before writing, with i18next {{name}} rewritten to ICU {name}. The other 5 are newly written here and are the ones needing a real loc pass: edit-row, status-feature-disabled, status-disabled, status-unavailable, administration-governance.

English only, like every other string in this package: chore(l10n): sync from Localization owns the other thirteen catalogs. i18n.test.ts keeps the two scans that are this PR's responsibility, from the family's shared __fixtures__/catalog-coverage: English parity with the source, and no catalog keeping an id the source has dropped.

__fixtures__/** is excluded from the rslib build alongside src/test/**: fixtures are data for the suites, not API, and they reach for devDependencies.

Verification

Guardrails suite 473 passing, 0 failing, jest-axe on all three rendering suites. tsc --noEmit and biome clean. Tests and stories typechecked separately, which CI does not do: clean in this PR's files, the remaining errors being #1138's own, now on main.

Open questions

  1. The mixed-scopes banner is a slot, not a typed prop. AL-575 names it, but the list composes banners through one statusBanner: ReactNode, and the banner's second line is a "Save as new" hint, which is builder copy. Say the word and it becomes mixedScopes={{ scopes, tools }} again.
  2. No onTrack prop, against Confluence §7.5. feat(apollo-react): guardrails component family under canvas #1138 has no telemetry seam, the two products' event taxonomies do not overlap (guardrails.reordered vs GUARDRAILS_REORDER_BUTTON_CLICKED), and wrapping the intents is what both adapters do anyway. Easy to add if you disagree.
  3. Agents' scoped-removal bug: GuardrailsListingSection swaps affectedScopes and affectedToolNames when building the remove-dialog payload. That code stays host-side either way, but the adapter has to decide whether to carry it.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 9, 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.2 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

@andreizdrali-uipath
andreizdrali-uipath force-pushed the feat/apollo-react-guardrail-list branch from eed9161 to 4ac1aa1 Compare September 9, 2026 13:35
@andreizdrali-uipath
andreizdrali-uipath changed the base branch from feat/apollo-react-guardrails-family to feat/apollo-react-guardrail-definitions-layer September 9, 2026 13:36
@andreizdrali-uipath andreizdrali-uipath added the dev-packages Adds dev package publishing on pushes to this PR label Sep 9, 2026
@andreizdrali-uipath
andreizdrali-uipath changed the base branch from feat/apollo-react-guardrail-definitions-layer to main September 9, 2026 13:58
@andreizdrali-uipath andreizdrali-uipath added dev-packages Adds dev package publishing on pushes to this PR and removed dev-packages Adds dev package publishing on pushes to this PR labels Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

📦 Dev Packages

🧹 Dev packages cleaned up after PR close.

Last updated: 2026-09-21 09:02:50 PT

Copilot AI lite review requested due to automatic review settings September 10, 2026 08:32
@andreizdrali-uipath
andreizdrali-uipath force-pushed the feat/apollo-react-guardrail-list branch from 4ac1aa1 to 246163c Compare September 10, 2026 08:32
@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 21, 2026, 07:28:28 AM
apollo-docs Ready Preview · Logs Sep 21, 2026, 07:28:28 AM
apollo-landing Ready Preview · Logs Sep 21, 2026, 07:28:28 AM
apollo-vertex Ready Preview · Logs Sep 21, 2026, 07:28:28 AM

@github-actions github-actions Bot added the size:XXL 1,000+ changed lines. label Sep 10, 2026

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

Resolve the two moderate issues affecting row-specific edit labels and name truncation.

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

Pull request overview

Adds the shared Apollo React guardrail list with host-owned actions, status handling, localization, and drag-and-drop reordering.

Changes:

  • Adds guardrail list rendering, notices, chips, callbacks, and reorder behavior.
  • Adds supporting forms, definitions, exports, fixtures, stories, and tests.
  • Updates dependencies and build configuration.
File summaries
File Reviewed change
pnpm-lock.yaml Locks the dnd-kit modifier dependency.
packages/apollo-wind/src/index.ts Updates Apollo Wind exports.
packages/apollo-wind/src/components/ui/textarea.tsx Provides the textarea UI component.
packages/apollo-wind/src/components/ui/select.tsx Provides the select UI component.
packages/apollo-wind/src/components/ui/info-tooltip.tsx Provides the info tooltip component.
packages/apollo-wind/src/components/ui/info-tooltip.test.tsx Tests the info tooltip.
packages/apollo-wind/src/components/ui/index.ts Exports UI components.
packages/apollo-wind/src/components/forms/validation-converter.ts Converts validation results.
packages/apollo-wind/src/components/forms/string-list-field.tsx Implements string-list form fields.
packages/apollo-wind/src/components/forms/metadata-form.stories.tsx Adds metadata form stories.
packages/apollo-wind/src/components/forms/index.ts Exports form components.
packages/apollo-wind/src/components/forms/form-schema.ts Defines form schemas.
packages/apollo-wind/src/components/forms/field-renderer.tsx Renders schema fields.
packages/apollo-react/src/test/setup.ts Configures React test setup.
packages/apollo-react/src/i18n/index.ts Provides localization integration.
packages/apollo-react/src/canvas/components/index.ts Exports canvas components.
packages/apollo-react/src/canvas/components/Guardrails/utils.ts Provides guardrail utilities.
packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts Loads guardrail definitions.
packages/apollo-react/src/canvas/components/Guardrails/types.ts Defines guardrail types.
packages/apollo-react/src/canvas/components/Guardrails/render-parameter-bridge.tsx Bridges parameter rendering.
packages/apollo-react/src/canvas/components/Guardrails/list-types.ts Defines list contracts.
packages/apollo-react/src/canvas/components/Guardrails/index.ts Exports guardrail APIs.
packages/apollo-react/src/canvas/components/Guardrails/i18n.test.ts Tests localization parity.
packages/apollo-react/src/canvas/components/Guardrails/guardrail-validator-form.tsx Renders validator forms.
packages/apollo-react/src/canvas/components/Guardrails/guardrail-list-utils.ts Resolves list identities and statuses.
packages/apollo-react/src/canvas/components/Guardrails/guardrail-list-utils.test.ts Tests list utilities.
packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.tsx Provides form layout.
packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.test.tsx Tests form layout.
packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.stories.tsx Adds form layout stories.
packages/apollo-react/src/canvas/components/Guardrails/form-schema-builder.ts Builds form schemas.
packages/apollo-react/src/canvas/components/Guardrails/definitions-wire.ts Defines wire-format definitions.
packages/apollo-react/src/canvas/components/Guardrails/definitions-parse.ts Parses definitions.
packages/apollo-react/src/canvas/components/Guardrails/definitions-parity.test.ts Tests definition parity.
packages/apollo-react/src/canvas/components/Guardrails/definitions-copy.test.ts Tests definition copy data.
packages/apollo-react/src/canvas/components/Guardrails/components/parameter-label.tsx Renders parameter labels.
packages/apollo-react/src/canvas/components/Guardrails/components/mixed-scopes-banner.tsx Renders mixed-scope notices.
packages/apollo-react/src/canvas/components/Guardrails/components/mixed-scopes-banner.test.tsx Tests mixed-scope notices.
packages/apollo-react/src/canvas/components/Guardrails/components/map-enum-field.tsx Renders enum fields.
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-chip.tsx Renders read-only status chips.
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-chip.test.tsx Tests status chips.
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-banner.tsx Renders status banners.
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-banner.test.tsx Tests status banners.
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-scope-selector.tsx Renders scope selection.
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-scope-selector.test.tsx Tests scope selection.
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-chip.tsx Renders guardrail chips.
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-chip.test.tsx Tests guardrail chips.
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-action-section.tsx Renders guardrail actions.
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-action-section.test.tsx Tests guardrail actions.
packages/apollo-react/src/canvas/components/Guardrails/components/field-shell.tsx Provides field shells.
packages/apollo-react/src/canvas/components/Guardrails/components/field-shell.test.tsx Tests field shells.
packages/apollo-react/src/canvas/components/Guardrails/components/enum-list-chips-field.tsx Renders enum list chips.
packages/apollo-react/src/canvas/components/Guardrails/builder-utils.ts Provides builder utilities.
packages/apollo-react/src/canvas/components/Guardrails/builder-utils.test.ts Tests builder utilities.
packages/apollo-react/src/canvas/components/Guardrails/builder-types.ts Defines builder types.
packages/apollo-react/src/canvas/components/Guardrails/__fixtures__/host-copy-baselines.ts Stores host copy baselines.
packages/apollo-react/src/canvas/components/Guardrails/__fixtures__/guardrail-list.fixtures.ts Provides list fixtures.
packages/apollo-react/src/canvas/components/Guardrails/__fixtures__/dnd-geometry.ts Provides reorder test geometry.
packages/apollo-react/src/canvas/components/Guardrails/__fixtures__/definitions-wire.fixtures.ts Provides definition fixtures.
packages/apollo-react/rslib.config.ts Excludes test fixtures from builds.
packages/apollo-react/package.json Adds package dependency configuration.
Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file

Suppressed comments (2)

packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-list-row.tsx:120

  • The remove controls also all expose the generic accessible name (Remove guardrail), leaving the target row ambiguous when users navigate by buttons. Add a localized name-bearing template and interpolate item.name here, matching the row-specific drag/edit labels.
            aria-label={labels.removeItem}

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

  • The name is a flex item, but it lacks min-w-0; Tailwind's truncate cannot shrink an auto-min-width flex item. Long guardrail names will therefore push the preview/status chips (and the row actions) out of the available width instead of ellipsizing. Add min-w-0 to this span so the chips remain visible.
          <span className="truncate text-sm font-medium">{item.name}</span>
  • Files reviewed: 95/96 changed files
  • Comments generated: 1
  • 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.6% 75.0% (114/152) 7.71 MB 29.84 MB +5.2 KB
@uipath/apollo-ui-icons 2.86 MB 6.96 MB ±0
@uipath/apollo-wind 469.4 KB 2.96 MB −20 B
@uipath/ap-chat 85.8% 43.94 MB 56.79 MB +299 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.

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Storybook visual diff

⚠️ Visual changes detected: 14 changed, 14 added (of 414 compared, 386 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, 07:46:30 AM

andreizdrali-uipath added a commit that referenced this pull request Sep 11, 2026
Review of #1140 against the #1107/#1138 threads (plan/review-2026-09-11 §4.3).

Tokens, the three themes Ben Schulz already pushed back on in #1107:
- The two BYO notices use `text-error`, wind's own error-text token, not
  `text-destructive`: the two resolve differently in several theme blocks of
  `tailwind.consumer.css`, and `FormFieldError` settled on `text-error`.
- `text-[11px]` twice becomes `text-xs`; the family has no arbitrary type sizes.
- The remove button keeps its four-scope destructive ghost override and its comment.
  A wind `ghost-destructive` variant is the better answer and widens this PR.

Accessibility, on the activatable row:
- The body is `role="button"`, so ARIA treats everything in it as presentational.
  The BYO notices and the description now carry ids and are named in the body's
  `aria-describedby`. The provider line, action badge and scopes stay presentational
  and the README says so, since naming all of them turns one announcement into a
  paragraph.
- A tooltipped body that is not activatable gets `tabIndex={0}`: Radix opens on focus
  as well as hover, so that is all the tooltip needed to stop being pointer-only.
- The notices keep `role="alert"` rather than the family banner's `role="status"`,
  with the reason in place: Flow announces them on mount today.

Claims match code:
- `capitalize` applies only to the raw `$actionType` branch, never to host-localized
  `formatAction` output, which is already cased for its locale.
- The sensors move into a `SortableRows` component next to the `DndContext`, so
  "with reorder off, no drag machinery mounts" is literally true rather than nearly:
  `useSensor`/`useSensors` are hooks and used to run for every list.
- `GuardrailListItemActionsContext.onEdit`/`onRemove` are documented as absent when
  the host passed no handler *or* the list is disabled.
- `GuardrailRowTooltipRenderer` is declared once in `list-types.ts` and exported,
  instead of the list indexing into the row's non-exported props.

README rows and tests cover each behaviour change: 91 tests across the five suites.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 11, 2026 08:19
@andreizdrali-uipath
andreizdrali-uipath force-pushed the feat/apollo-react-guardrail-list branch from 246163c to 4eb941b Compare September 11, 2026 08:19
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.

🔵 Needs a closer look

Two moderate findings (1 vote each) remain unresolved: conflicting aria-describedby ownership and omitted status/administration chip announcements.

Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file

Suppressed comments (2)

packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-list-row.tsx:279

  • When rowActivatesEdit is combined with renderRowTooltip (the Agents configuration), both the activatable body and Radix's TooltipTrigger asChild try to own aria-describedby. The as-child composition does not concatenate those IDs, so one value wins and screen readers lose either the BYO notice/description or the tooltip description. Preserve/merge both sets of ids, or put the tooltip trigger on a separate wrapper that does not own the activatable body's attribute.
          <TooltipProvider>
            <Tooltip>
              <TooltipTrigger asChild>{body}</TooltipTrigger>
              <TooltipContent>{tooltip}</TooltipContent>

packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-list-row.tsx:166

  • When rowActivatesEdit is enabled, the row body has role="button", so its child chips are treated as presentational. These status and administration labels are not included in aria-describedby, which leaves screen readers announcing only the edit label and optional description while omitting the warning/error or governance state. Include the chip labels in an associated accessible description (or expose them outside the activatable body) for this valid prop combination.
    const describedBy = activatable
      ? [
          state.byoDisabled ? byoDisabledId : undefined,
          state.byoUnavailable ? byoUnavailableId : undefined,
          item.description ? descriptionId : undefined,
        ]
  • Files reviewed: 112/113 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

andreizdrali-uipath added a commit that referenced this pull request Sep 11, 2026
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.
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>
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.
andreizdrali-uipath added a commit that referenced this pull request Sep 14, 2026
…AL-578]

The row was `px-1 py-1.5`, so its hover box had 6px above and below against
4px left and right. `p-1` everywhere, matching the 4px `GuardrailListRow`
settled on in #1140 (`6ed464b9`), so entries in the two sections that render
one above the other are the same height.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
andreizdrali-uipath added a commit that referenced this pull request Sep 14, 2026
…578]

Apollo maps `--accent` to `--surface-hover` and `--muted` to `--surface-overlay`,
the raised panel this section sits on, so a hovered row was painting itself the
colour of its own background. The palette item in #1147 already uses `accent`
and the list row in #1140 moves with this.

The README gains the rule the three rows share, in the wording #1140 carries, so
the two branches merge to one copy: hover is never a prop, an element that is
itself a control gates it on being enabled and pairs it with a cursor and a ring,
and a row that merely contains controls highlights unconditionally.

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.

🔵 Needs a closer look

Two moderate findings remain for localized reorder announcements and translated Edit labels; the README coverage count also needs correction.

Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file

Suppressed comments (3)

packages/apollo-react/src/canvas/components/Guardrails/README.md:275

  • The list now declares 18 message ids: remove-row was added alongside the original 17, and 13 of those ids are present in every translated catalog. This paragraph still says “Twelve of the seventeen,” so the public localization contract understates the harvested coverage.
the shared canvas catalog); `labels` overrides any of them and wins over the catalog. Twelve
of the seventeen are harvested from Flow's canvas catalog into all 12 translated locales, so

packages/apollo-react/src/canvas/components/Guardrails/guardrail-list.tsx:153

  • The keyboard reorder path uses dnd-kit's default live-region announcements, which announce the sortable active.id and English text. In the supported Flow shape that id is an opaque value such as g1, so a screen-reader user hears an internal identifier rather than the row name, and the instructions remain untranslated even when the list labels are localized. Provide localized accessibility instructions/announcements that resolve the active and over ids back to row names and positions.
    packages/apollo-react/src/canvas/components/Guardrails/i18n.ts:552
  • editRow is now the accessible name used by every Edit button, but this message id is absent from all 12 translated catalogs while the existing editItem id is translated in every locale. As a result, localized users hear the Edit action in English for every row. Please add translations for this row-name template (or keep a localized fallback strategy) before making it the default action label.
  • Files reviewed: 110/111 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

andreizdrali-uipath added a commit that referenced this pull request Sep 15, 2026
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.
andreizdrali-uipath added a commit that referenced this pull request Sep 15, 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.
andreizdrali-uipath added a commit that referenced this pull request Sep 15, 2026
…n [AL-579]

Wind's `AlertDescription` is unconditionally `mt-1`, which assumes an
`AlertTitle` sits above it, while `alertVariants` pins the icon at
`[&>svg]:top-3.5`. The app-picker-unavailable alert has no title, so its text
rendered 4px below the icon.

Same `mt-0` call-site workaround as `1b6792db` on #1161, `12a2d420` on #1140 and
`e7e12307` on #1147. This file was left out of that round because it belonged to
no open ticket then; AL-579 owns it now. The real fix is one line in wind's
`alertVariants`, which Andrei is opening separately, and which would let all four
workarounds be deleted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
andreizdrali-uipath added a commit that referenced this pull request Sep 16, 2026
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.
andreizdrali-uipath added a commit that referenced this pull request Sep 16, 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.
andreizdrali-uipath added a commit that referenced this pull request Sep 16, 2026
…L-574]

A read-only status label for a guardrail row: definition status in the palette,
governance origin in the centralized section. It lands here rather than in each
leaf because #1140, #1147 and #1161 all branch from this layer and were each
carrying a byte-identical copy, kept in step by hand.

Deliberately not `GuardrailChip`, which wraps a Radix `Toggle`: these are
labels, and rendering them as toggles would put fake buttons in the tab order.
It is a `<span>` composed from wind's exported `badgeVariants` rather than the
`Badge` component, which renders a `<div>`, because the palette entry puts these
inside its `<button>` where flow content is invalid.

`GUARDRAIL_CHIP_GEOMETRY` is extracted from `guardrail-chip.tsx` so the
interactive and read-only pills stay one system rather than drifting apart.

Four tones, one per definition status plus the green both products already give
a bring-your-own origin. Nothing in this PR renders it yet; the three leaves do.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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. `__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-list branch from bceb310 to d9f0643 Compare September 16, 2026 10:45
Copilot AI review requested due to automatic review settings September 16, 2026 10:45

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 unresolved.

Get a fresh assessment by requesting 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-list-row.tsx:105

  • actionUnknown is documented as the label for an action that is missing or unrecognized, but this branch renders every non-null $actionType verbatim. A persisted/future discriminator such as review will therefore appear as raw review instead of the documented Unknown label (and the current test only covers the missing-action case). Check the action against the supported discriminator set before rendering it, or make the API explicitly promise raw passthrough and remove the unused fallback contract.
    const actionContent = formatAction
      ? formatAction(item)
      : (item.action?.$actionType ?? labels.actionUnknown);

packages/apollo-react/src/canvas/components/Guardrails/guardrail-list-utils.ts:60

  • When the definitions request has completed with an empty array (or host-side filtering removes every definition), this condition treats it as still loading forever. A BYO row then never receives the required Unavailable notice or status chip. definitions.length cannot distinguish loading from a successfully loaded empty result; pass an explicit loading/loaded signal or use undefined as the loading sentinel and suppress this only while loading.
  const byoUnavailable = isByo && definitions.length > 0 && definition === undefined;
  const byoDisabled = isByo && definition?.status === 'Disabled';

packages/apollo-react/src/canvas/locales/en.json:290

  • These guardrails.list.* entries are only present in en.json; none of the other canvas catalogs contain them. Because useSafeLingui falls back to the English descriptor when the active catalog lacks an id, every list title/action/status label is English in localized providers (including the Japanese story), contrary to the stated 12-locale harvest. Add the corresponding locale entries before relying on the localized list.
  • Files reviewed: 40/41 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread packages/apollo-react/src/canvas/components/Guardrails/i18n.test.ts Outdated
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. `__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 10:55

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

Three unresolved moderate findings remain in row accessibility and definitions state/key handling.

Get a fresh assessment by requesting another Copilot review.

Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file

Suppressed comments (2)

packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-list-row.tsx:190

  • When rowActivatesEdit and renderRowTooltip are both enabled (the documented Agents setup), this explicit aria-describedby takes precedence over Radix's tooltip description. The list intentionally puts only chips, BYO notices, and the description in describedBy, so the tooltip-only provider and scopes are never announced to screen readers; they remain visual/hover-only. Merge an accessible description for those tooltip-only fields (or merge the Radix description id instead) and cover the combined configuration.
          ...(describedBy ? { 'aria-describedby': describedBy } : {}),

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

  • This key is documented as a content comparison, but JSON.stringify preserves object insertion order. Two equivalent headers records constructed in different orders therefore produce different keys, causing the effect to abort/refetch and temporarily hide the settled definitions even though the request inputs are unchanged. Canonicalize the header entries before serializing (or use a stable serializer).
  • Files reviewed: 40/41 changed files
  • Comments generated: 1
  • Review effort level: Lite

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

Two moderate issues remain in the definitions hook: stale data after re-enabling a context and stale transport errors with host-provided definitions.

Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file

Suppressed comments (4)

packages/apollo-react/src/canvas/components/Guardrails/README.md:293

  • These counts do not match the implementation: GUARDRAIL_LIST_EN_MESSAGES and en.json define 18 guardrails.list.* IDs, and the five IDs listed as newly written leave 13 inherited messages, not 12 of 17. Please update the documentation so the localization contract is accurate.
of the seventeen take their English from Flow's canvas catalog, so the wording is what the
product already ships; the other five have no host equivalent and are newly written here:
`edit-row`, `status-feature-disabled`, `status-disabled`, `status-unavailable`,
`administration-governance`.

packages/apollo-react/src/canvas/components/Guardrails/guardrail-list.stories.tsx:255

  • This story is labeled as Japanese-localized, but canvas/locales/ja.json has no guardrails.list.* entries from this PR. useSafeLingui therefore falls back to the English defaults for every list label, so the story neither displays Japanese nor verifies a translated catalog. Either supply a story-local Japanese catalog/overrides or rename the story and description to make the English fallback explicit.
/** Chrome strings resolved from the canvas lingui catalog (Japanese). */
export const Localized: Story = {
  args: { ...Default.args },
  render: (args) => (
    <ApI18nProvider component="canvas" locale="ja">

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

  • Re-enabling a previously fetched context can expose stale definitions until the new request resolves. Disabling via ctx === null (or switching to a host-provided payload) leaves settled stamped with the old request key; when the same context is enabled again, isCurrent is immediately true and loading is false on the first render, so consumers can render the old tenant/configuration as current. Clear or invalidate settled in this disabled branch, and add a regression test for disable → re-enable with the same context.
    packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:137
  • When options.definitions is supplied after a request failure, this still returns the previous transport error because error is derived solely from settled and isCurrent. The hook then exposes valid host-provided definitions together with an obsolete error, so consumers that render the status banner on error show a false failure. The host-provided path should force error to null (and ideally cover the transition from a failed fetch in the hook test).
  • Files reviewed: 40/41 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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 findings remain unresolved, covering action fallback, chip sizing, and header-key canonicalization.

Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file

Suppressed comments (4)

packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-list-row.tsx:105

  • actionUnknown is documented as the fallback for both missing and unrecognized actions, but this expression only falls back when the action is absent. A persisted or future $actionType such as archive is rendered verbatim instead of using the localized Unknown label; check the value against the supported action discriminators before rendering it (or let the host formatter handle that case).
    const actionContent = formatAction
      ? formatAction(item)
      : (item.action?.$actionType ?? labels.actionUnknown);

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

  • This story describes governance as an "origin", but the public API deliberately reserves that term for the BYO-vs-managed axis; the local-vs-governance axis is named administration throughout list-types.ts and the Guardrails README. Calling it an origin can lead consumers to conflate these two different chips. Please use "governance administration" here.
Read-only status label for a guardrail row: definition status in the palette, governance
origin in the centralized section. It is a span carrying the wind badge classes, not a

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

  • badgeVariants already contributes py-0.5 (and the text-xs line height is 1rem), so adding only h-5 leaves the chip's content/padding taller than its fixed 20px box and the label can overflow vertically. Override the vertical padding or remove the fixed height so the advertised compact geometry is valid.
        'h-5',

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

  • The request key treats headers as an order-sensitive JSON string even though HTTP headers are unordered. Two semantically identical header objects with different insertion order produce different keys, abort the settled request, and refetch; a host that rebuilds headers in a different order can therefore churn or fail to settle. Canonicalize the header entries before serializing the key.
  • Files reviewed: 40/41 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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 remain in definition state and row fallback/accessibility behavior.

Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file

Suppressed comments (4)

packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-list-row.tsx:102

  • actionUnknown is documented as the fallback for an unrecognized action, but this only falls back when $actionType is absent. Because GuardrailListItem.action.$actionType is intentionally widened to string, a legacy or newer value such as futureAction is rendered verbatim (and capitalized) instead of using the localized Unknown label. Validate the discriminator against the supported action types before using it as the default badge content.
    const actionContent = formatAction
      ? formatAction(item)
      : (item.action?.$actionType ?? labels.actionUnknown);

packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-list-row.tsx:228

  • When rowActivatesEdit is enabled, this notice is rendered inside the row body's role="button". A button's descendants are presentational in the accessibility tree, so the nested role="alert" is not a reliable live-region announcement for the Agents row; it only becomes text referenced by aria-describedby. Render the alert as a sibling outside the interactive body (while keeping it in aria-describedby) or provide a separate live region so a newly disabled/unavailable configuration is announced.
        {state.byoDisabled && (
          <div
            id={byoDisabledId}
            className="whitespace-normal break-words text-xs text-error"
            role="alert"

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

  • The public API distinguishes administration (local vs governance) from origin (BYO vs UiPath-managed), but this Storybook description calls the centralized-section value “governance origin.” That terminology can mislead consumers about which axis this chip represents; describe it as governance administration instead.
Read-only status label for a guardrail row: definition status in the palette, governance
origin in the centralized section. It is a span carrying the wind badge classes, not a
control, so it is safe inside a button and stays out of the tab order. Use

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

  • The disabled effect only aborts the request and clears inFlight; it leaves the last settled payload stamped with the same request key. After fetching for context K, disabling with null and then re-enabling K makes that old payload current again, so stale definitions are exposed while the new request starts (and loading is initially false), despite the hook contract saying disabling clears fetched state. Reset the settled result in this branch before allowing the hook to be enabled again.
  • Files reviewed: 40/41 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

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 payload safety, definition state, and Agents row rendering.

Get a fresh assessment by requesting 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-list-row.tsx:253

  • renderRowTooltip only wraps this body; it does not replace the content rendered here. As a result, the Agents configuration shown in the README/story still renders the description inline and in the tooltip (and the unconditional provider/scopes lines below have the same effect), rather than preserving Agents' tooltip-only row layout. Please add an explicit details/rendering mode or slot that lets the host suppress these inline lines when it supplies the tooltip.
        {item.description && (
          <div id={descriptionId} className="truncate text-xs text-muted-foreground">
            {item.description}
          </div>
        )}

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

  • This story calls the local-vs-governance status axis a “governance origin,” but origin already denotes the separate BYO-versus-UiPath provenance axis in this family. Use the public API's administration terminology here so the story does not teach consumers the wrong concept.
Read-only status label for a guardrail row: definition status in the palette, governance
origin in the centralized section. It is a span carrying the wind badge classes, not a
control, so it is safe inside a button and stays out of the tab order. Use

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

  • When the hook is disabled, this branch only makes the old settled value unreachable while requestKey is empty; it never clears it. If the same context is enabled again, the previous request's definitions are treated as current (settled.key === requestKey), so the hook renders stale rows and reports loading: false until the refetch effect runs, and a failed refetch can keep those rows. Reset the settled result here as the documentation and disabled-state contract promise.
    packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:137
  • When options.definitions is supplied after a prior fetch for the same ctx has failed, enabled becomes false but isCurrent remains true, so this returns the old transport error alongside the host-provided definitions. Consumers that render error as a load-failure banner can show a stale failure even though the override path succeeded; gate this read with enabled or clear the settled error when switching to provided data.
  • Files reviewed: 40/41 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread packages/apollo-react/src/canvas/components/Guardrails/definitions-parse.ts Outdated

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 definition lookup safety and moderate stale-state and accessibility issues must be addressed.

Get a fresh assessment by requesting 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-status-chip.stories.tsx:13

  • This story calls the local-versus-governance axis “origin”, but the public API deliberately names that axis administration; “origin” is reserved for BYO versus UiPath-managed provenance. Keeping the story terminology aligned with GuardrailListAdministration avoids teaching consumers to map the wrong concept.
origin in the centralized section. It is a span carrying the wind badge classes, not a

packages/apollo-react/src/canvas/components/Guardrails/guardrail-list.tsx:180

  • The drag handle interpolates the raw name, unlike the edit/remove controls which trim it and fall back for blank names. For a row whose required name is whitespace or empty, this produces an effectively unnamed control such as Reorder guardrail , so keyboard and screen-reader users cannot identify which row they are moving. Reuse the same non-empty name fallback here (or add a localized generic reorder label).
    packages/apollo-react/src/canvas/components/Guardrails/use-guardrail-definitions.ts:207
  • This branch only makes the settled result unreachable while requestKey === ''; it does not clear settled. If the hook is disabled and then enabled again for the same context, settled.key === requestKey makes the old result current immediately, so loading is false and a failed refresh can leave stale definitions visible. Reset settled here to honor the documented "disabling clears the fetched state" behavior.
    packages/apollo-react/src/canvas/locales/en.json:304
  • The PR description says this adds 17 guardrails.list.* ids (12 harvested plus 5 new), but the shipped source/catalog declare 19 ids from title through administration-governance. Please reconcile the documented string inventory with the public label surface and localization scope.
  • Files reviewed: 40/41 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread packages/apollo-react/src/canvas/components/Guardrails/definitions-enrich.ts Outdated

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

🟡 Changes recommended

Unknown action types are rendered verbatim instead of using the documented fallback, and new examples contain inaccurate documentation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 Medium severity · 2 Low severity

Open (4)
Resolved since last review (3)
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

Comment thread packages/apollo-react/src/canvas/components/Guardrails/README.md
Add `GuardrailList`, the applied-guardrails list both products render under a
tool or an agent: rows with their lifecycle and status chips, BYO notices,
per-row edit and remove, drag reordering, and the empty state.

The host filters and the list renders. Items arrive already resolved against
their definitions, every callback is an intent, and `renderItemActions`,
`statusBanner` and `addSlot` are slots rather than product branches, so no
telemetry, flag or product type crosses the boundary. `GuardrailListItem` is a
structural mirror of what both products already hold.

Reordering is dnd-kit with `verticalListSortingStrategy`, restricted to the
vertical axis and the parent element (`@dnd-kit/modifiers`, the one new
dependency). The row transform is `CSS.Translate`, never `CSS.Transform`:
`useSortable` derives its layout transform from the row's before and after
rects, so with variable row heights (a description line, a BYO notice)
`CSS.Transform` emits a `scaleY` that visibly squashes or stretches the row's
text mid-animation. The strategy never produces a scale of its own, so dropping
it costs nothing. Verified against the installed `@dnd-kit/sortable` source,
which the published docs do not cover.

Rows highlight on hover unconditionally, like wind's `TableRow`: a row is a
hover target whether or not clicking it opens the editor. `rowActivatesEdit`
restores the legacy click-to-edit behaviour, and only then does the row body
carry `cursor-pointer` and its own focus ring, since only then is it a control.
The tint is `accent`, Apollo's hover surface (`--accent: var(--surface-hover)`),
not `muted`, which is `surface-overlay`, the panel the list usually sits on and
therefore invisible against it. The family-wide rule is in the README.

Because the row paints that tint, the drag handle carries no negative margin:
`p-1` gives the row 4px, which is exactly the reach of the handle's
`ring-offset-2` focus ring, so pulling the handle out to align its glyph with
the row's leading edge put its hover surface and its ring outside the tinted
box. The glyph sits an icon button's worth of padding inside the edge instead.

`byoChip` badges a bring-your-own row beside its name, opt-in like `previewChip`
because it is a product decision rather than something inferable from
`state.isByo`: Agents badges these rows (#6275, AL-590), Flow does not. Without
it a host adopting the list lost the chip, since the row builds its chips
internally from `getGuardrailListChips`, whose ids are a closed union, and the
name area takes no slot. It renders before `previewChip`, because a BYO row is
also a built-in validator and provenance reads before lifecycle, in the
`success` tone the chip already documents as the colour both products give BYO.
Agents' italic provider line from the same ticket is not included: that restyles
the shared row rather than adding to it.

Seventeen `guardrails.list.*` lingui ids. Twelve take their English from Flow's
canvas catalog, so the wording is what the product already ships; the other
five have no host equivalent and are newly written here (`edit-row`,
`status-feature-disabled`, `status-disabled`, `status-unavailable`,
`administration-governance`), so they are the ones needing a real loc pass
rather than a lookup. 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 reader,
and the stories carry no Japanese example: with the translations gone it would
render English and claim otherwise.

The "Preview" lifecycle chip takes the family's `info` tone, added to
`GuardrailStatusChip` on #1139 for this and the palette: blue is what both
products already give that chip (Flow's `InfoBadge tone="preview"`, Agents'
`PreviewChip` on `semantic.colorInfoBackground`), where this row shipped it
neutral grey.

`GuardrailStatusBanner` and `MixedScopesBanner` become public so a host can
compose the `statusBanner` slot, and each gains a story. `MixedScopesBanner`
also gains the `mt-0` the title-less alert fix should have given it: wind's
`AlertDescription` is unconditionally `mt-1`, which assumes an `AlertTitle`
above it, so a title-less alert renders its text 4px below its icon. The same
hunk is on apollo-ui#1147 and apollo-ui#1161 so the three merge without a
conflict; the real fix is one line in wind's `alertVariants`.

`__fixtures__/**` is excluded from the rslib build alongside `src/test/**`:
fixtures are data for the suites, not API, and they reach for devDependencies.

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 review overview

🟡 Changes recommended

The new pointer-based drag path lacks coverage for activation and mouse/touch drop behavior.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Low severity

Open (1)
Resolved since last review (4)
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

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