feat(apollo-react): centralized guardrails section and read-only details [AL-578] - #1161
Conversation
|
Apollo Coded App preview deployments are ready.
|
Dependency License Review
License distribution
Excluded packages
|
📦 Dev Packages🧹 Dev packages cleaned up after PR close. Last updated: 2026-09-23 08:34:45 PT |
There was a problem hiding this comment.
🟡 Changes recommended
Resolve the critical key-collision issue and the two moderate UI rendering issues.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds shared read-only centralized guardrails list and details UI for Apollo React, with definition-aware rendering, localization, status messaging, and host-controlled shells.
Changes:
- Adds centralized guardrail types, utilities, components, and exports.
- Adds read-only sections, details, configuration displays, stories, and documentation.
- Updates form integrations, chat rendering, localization catalogs, tests, and dependencies.
Review findings: Critical (2 votes): guardrail key collisions in centralized-guardrail-utils.ts. Moderate (1 vote each): interactive tooltip nesting in parameter-label.tsx, and unstable renderer identities in chat-message-content.tsx.
File summaries
| File | Reviewed change |
|---|---|
pnpm-lock.yaml |
Dependency lockfile updates |
packages/apollo-wind/src/index.ts |
Public package exports |
packages/apollo-wind/src/components/ui/textarea.tsx |
Textarea component |
packages/apollo-wind/src/components/ui/select.tsx |
Select component |
packages/apollo-wind/src/components/ui/info-tooltip.tsx |
Info tooltip component |
packages/apollo-wind/src/components/ui/info-tooltip.test.tsx |
Tooltip tests |
packages/apollo-wind/src/components/ui/info-tooltip.stories.tsx |
Tooltip stories |
packages/apollo-wind/src/components/ui/index.ts |
UI exports |
packages/apollo-wind/src/components/ui/form-field.tsx |
Form field layout and labels |
packages/apollo-wind/src/components/ui/form-field.test.tsx |
Form field tests |
packages/apollo-wind/src/components/forms/validation-converter.ts |
Validation conversion |
packages/apollo-wind/src/components/forms/validation-converter.test.ts |
Validation tests |
packages/apollo-wind/src/components/forms/string-list-field.tsx |
String-list field |
packages/apollo-wind/src/components/forms/schema-serializer.ts |
Schema serialization |
packages/apollo-wind/src/components/forms/schema-serializer.test.ts |
Serialization tests |
packages/apollo-wind/src/components/forms/rules-engine.ts |
Form rules engine |
packages/apollo-wind/src/components/forms/README.md |
Forms documentation |
packages/apollo-wind/src/components/forms/metadata-form.stories.tsx |
Metadata form stories |
packages/apollo-wind/src/components/forms/index.ts |
Forms exports |
packages/apollo-wind/src/components/forms/form-schema.ts |
Form schema definitions |
packages/apollo-wind/src/components/forms/form-designer.tsx |
Form designer |
packages/apollo-wind/src/components/forms/demo-mocks.ts |
Demo form mocks |
packages/apollo-wind/src/components/forms/custom-controls.stories.tsx |
Custom control stories |
packages/apollo-react/src/test/setup.ts |
Test setup |
packages/apollo-react/src/material/components/ap-chat/components/message/chat-message-content.tsx |
Locale-aware chat rendering |
packages/apollo-react/src/material/components/ap-chat/components/message/chat-message-content.test.tsx |
Chat rendering tests |
packages/apollo-react/src/i18n/index.ts |
Internationalization exports |
packages/apollo-react/src/canvas/locales/ru.json |
Russian translations |
packages/apollo-react/src/canvas/components/index.ts |
Canvas component exports |
packages/apollo-react/src/canvas/components/Guardrails/utils.ts |
Guardrail utilities |
packages/apollo-react/src/canvas/components/Guardrails/use-metadata-form-bridge.ts |
Metadata form bridge |
packages/apollo-react/src/canvas/components/Guardrails/types.ts |
Guardrail types |
packages/apollo-react/src/canvas/components/Guardrails/render-parameter-bridge.tsx |
Parameter rendering bridge |
packages/apollo-react/src/canvas/components/Guardrails/index.ts |
Guardrail exports |
packages/apollo-react/src/canvas/components/Guardrails/i18n.test.ts |
Localization tests |
packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.tsx |
Guardrail form layout |
packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.test.tsx |
Layout tests |
packages/apollo-react/src/canvas/components/Guardrails/guardrail-form-layout.stories.tsx |
Layout stories |
packages/apollo-react/src/canvas/components/Guardrails/definitions-wire.ts |
Wire definitions |
packages/apollo-react/src/canvas/components/Guardrails/definitions-copy.test.ts |
Definition copy tests |
packages/apollo-react/src/canvas/components/Guardrails/components/parameter-label.tsx |
Parameter labels and tooltips |
packages/apollo-react/src/canvas/components/Guardrails/components/mixed-scopes-banner.tsx |
Mixed-scopes banner |
packages/apollo-react/src/canvas/components/Guardrails/components/mixed-scopes-banner.test.tsx |
Banner tests |
packages/apollo-react/src/canvas/components/Guardrails/components/map-enum-field.tsx |
Enum field mapping |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-chip.tsx |
Status chip |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-chip.test.tsx |
Status chip tests |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-banner.tsx |
Status banner |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-banner.test.tsx |
Status banner tests |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-scope-selector.tsx |
Scope selector |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-chip.tsx |
Guardrail chip |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-chip.test.tsx |
Guardrail chip tests |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-action-section.tsx |
Action section |
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-action-section.test.tsx |
Action section tests |
packages/apollo-react/src/canvas/components/Guardrails/components/field-shell.tsx |
Field shell |
packages/apollo-react/src/canvas/components/Guardrails/components/field-shell.test.tsx |
Field shell tests |
packages/apollo-react/src/canvas/components/Guardrails/components/enum-list-chips-field.tsx |
Enum list field |
packages/apollo-react/src/canvas/components/Guardrails/components/centralized-guardrail-row.tsx |
Centralized guardrail rows |
packages/apollo-react/src/canvas/components/Guardrails/components/centralized-guardrail-parameters.tsx |
Read-only parameters |
packages/apollo-react/src/canvas/components/Guardrails/components/centralized-guardrail-origin-chip.tsx |
Origin indicator |
packages/apollo-react/src/canvas/components/Guardrails/centralized-types.ts |
Centralized guardrail contracts |
packages/apollo-react/src/canvas/components/Guardrails/centralized-guardrails-section.tsx |
Read-only guardrail section |
packages/apollo-react/src/canvas/components/Guardrails/centralized-guardrail-details.tsx |
Guardrail details |
packages/apollo-react/src/canvas/components/Guardrails/centralized-guardrail-details.test.tsx |
Details and accessibility tests |
packages/apollo-react/src/canvas/components/Guardrails/builder-utils.ts |
Builder utilities |
packages/apollo-react/src/canvas/components/Guardrails/builder-types.ts |
Builder types |
packages/apollo-react/src/canvas/components/Guardrails/__fixtures__/definitions-wire.fixtures.ts |
Definition fixtures |
packages/apollo-react/src/canvas/components/Guardrails/__fixtures__/catalog-coverage.ts |
Catalog coverage fixture |
packages/apollo-react/package.json |
Package metadata and scripts |
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
packages/apollo-react/src/canvas/components/Guardrails/components/parameter-label.tsx:35
InfoTooltiprenders a real<button>, so placing it insideLabelcreates an interactive labelable descendant and makes clicks on the tooltip ambiguously activate the associated form control. This also contradicts the sharedFormFieldLabelpattern, which deliberately renders the tooltip trigger beside the label (packages/apollo-wind/src/components/ui/form-field.tsx:61-69). Keep the tooltip outside the<Label>for the normal field-label path, while retaining the current text-header path.
packages/apollo-react/src/material/components/ap-chat/components/message/chat-message-content.tsx:35getApolloMessageRendererscreates new arrow-function component types on everyAutopilotChatMessageContentComponentrender. Because the result is used as<ApolloMessageRenderer>, any parent rerender changes the element type and unmounts/remounts the tool-call or tree renderer, resetting local state such as an expanded tool call and losing its DOM state. Define these renderer components at module scope (passinglocaleas a prop) or memoize a stable renderer table instead of creating component identities during render.
- Files reviewed: 112/113 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.
📊 Coverage + size by packagePer-package coverage and bundle size on this PR. New-line coverage = of the source lines this PR adds or changes, the % hit by tests.
"Coverage" is each package's own |
Storybook visual diffBaseline is the deployed main Storybook, so changes merged to main after this branch was last updated can also appear here. Logs Updated (PT): Sep 23, 2026, 02:57:47 AM |
There was a problem hiding this comment.
🔵 Needs a closer look
Four moderate review findings remain unresolved, covering threshold-map handling, decimal input editing, label/tooltip structure, and chat renderer identity.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (4)
Previously missed (2) — in code that hasn't changed since the last review.
packages/apollo-react/src/canvas/components/Guardrails/centralized-guardrail-utils.ts:257
- The
hascheck treats a parameter entry whosevalueisnull/undefinedas configured, so thekeySourcerow is consumed even thoughtoThresholdRowhas no threshold value to render. A connector payload with{ id: 'thresholds', value: undefined }(ornull) will therefore hide the selected entities entirely. Check that the value is a plain threshold-map object before adding itskeySourcetoconsumedIds(and add a regression test for an explicit empty value).
packages/apollo-react/src/material/components/ap-chat/components/message/chat-message-content.tsx:210 getApolloMessageRendererscreates new inline component functions on every render, so theApolloMessageRenderertype changes whenever this component re-renders. React consequently remounts the tool/tree renderer and loses state such as an expandedApToolCall; the previously existing test covered this behavior but is removed in this change. Memoize the renderer list bylocale(unconditionally, before the early returns) or move these renderer components to module scope so their identity is stable.
packages/apollo-react/src/canvas/components/Guardrails/components/map-enum-field.tsx:74
- Clearing or editing a decimal threshold can be coerced to
0before the user finishes typing: an empty intermediate value and values such as0.both pass throughNumber.parseFloat(...) || 0, immediately replacing the controlled input. This makes ordinary keyboard editing of fractional thresholds unreliable and can persist an unintended zero; retain the in-progress string (or otherwise defer numeric coercion until a complete value is available) and convert it at commit/validation time.
packages/apollo-react/src/canvas/components/Guardrails/components/parameter-label.tsx:25 InfoTooltiprenders a real<button>, so placing it inside the<label>creates invalid label content and makes clicking the tooltip trigger ambiguously activate the associated control.FormFieldLabelalready avoids this by rendering the label and tooltip as siblings; use the same wrapper pattern here while keeping the tooltip inside the text-header branch where no label is rendered.
- Files reviewed: 112/113 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
Three moderate findings remain unresolved around parameter handling, row-key collisions, and the info-button accessible label.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (5)
Previously missed (2) — in code that hasn't changed since the last review.
packages/apollo-react/src/canvas/components/Guardrails/centralized-guardrail-utils.ts:261
valuesById.has(...)only checks that the parameter entry exists, so amap-enumwhose value is explicitlynullorundefinedstill consumes itskeySourcelist. That makes a selected entity list render as a threshold row with unset values instead of remaining a standalone value row, contrary to the documented “only when it has a value” rule. Check the stored map value fornull/undefinedbefore adding the source id toconsumedIds.
packages/apollo-react/src/canvas/components/Guardrails/components/centralized-guardrail-row.tsx:120- This comment says the no-handler path renders a disabled button, but the branch immediately below returns a
<div>specifically so the row is plain text and not a control. Please update the comment to describe the current behavior; otherwise future changes may incorrectly restore the disabled-button behavior the story and tests reject.
packages/apollo-react/src/canvas/components/Guardrails/centralized-guardrail-utils.ts:47
executionStageis intentionally an open string and bothvalidatorand a BYOnamecan contain-, so concatenating these fields with-is not collision-safe. For example, a built-in{ validator: 'a-b', executionStage: 'c' }and{ validator: 'a', executionStage: 'b-c' }produce the same React key, which can trigger duplicate-key warnings and make React reuse the wrong row after a policy update. Encode the validator, stage, origin kind, and BYO name as separate tuple fields instead of delimiter concatenation.
const origin = guardrail.isByo ? (guardrail.name ?? 'byo') : 'builtin';
return `${guardrail.validator}-${guardrail.executionStage}-${origin}`;
packages/apollo-react/src/canvas/components/Guardrails/centralized-guardrails-section.tsx:114
- This icon-only button uses the full popover body as its accessible name, so a screen reader announces the long policy sentence as the control name instead of identifying the control as “More information.” The wind
FormFieldLabelpattern keeps these separate (contentversustooltipAriaLabel); add a localized centralized info-button label and use it foraria-labelhere while leavinglabels.infoas the popover content.
aria-label={labels.info}
packages/apollo-react/src/canvas/components/Guardrails/components/guardrail-status-chip.tsx:17
- The new
successtone is not exercised by the existing parameterized test, which only coversneutral,warning, anderror. Since this mapping selects a distinctbadgeVariantsclass, a regression could make the BYO origin/status chip use the wrong styling while the suite remains green; add asuccesscase (and include it in the relevant accessibility render if desired).
- Files reviewed: 112/113 changed files
- Comments generated: 0 new
- Review effort level: Lite
The legacy entries the list replaces highlight on hover, which is what makes a clickable row readable as clickable. The tint sits on the row container rather than on the body that carries `role="button"`, because the handle and the actions are siblings of it and legacy highlights the entire entry; the negative margin pairs with the padding so the tinted area extends past the content without moving it. Gated on `rowActivatesEdit`, so a row that does nothing when clicked does not advertise that it does. Same classes as the centralized section's row in #1161, so the two families hover alike. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The tint landed gated on `rowActivatesEdit`, which neither host passes yet, so in practice no row ever highlighted. A row is a hover target whether or not clicking it opens the editor: it is what you aim the drag handle and the row actions at, and the centralized section's rows in #1161 highlight the same way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
One or more issues must be addressed before approval.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (4)
packages/apollo-react/src/canvas/components/Guardrails/centralized-guardrail-utils.ts:47
- This key is built by joining unconstrained wire strings with
-, so distinct(validator, executionStage, name)tuples can collide (for examplea-b+canda+b-c). If both rows are present, React can reuse the wrong row DOM/state. Encode the tuple rather than using an ambiguous delimiter.
return `${guardrail.validator}-${guardrail.executionStage}-${origin}`;
packages/apollo-react/src/canvas/components/Guardrails/centralized-guardrail-utils.ts:257
valuesById.has()is also true when the connector sends a map parameter withvalue: nullorvalue: undefined. That makes this consume thekeySourcelist andtoThresholdRowrenders selected entities with—, whereas an unset map should leave the entity-list row visible (the same behavior as when the map parameter is absent). Check that the map value is non-null/defined before adding its key source toconsumedIds.
parameterDefinition.keySource !== undefined &&
valuesById.has(parameterDefinition.id) &&
rendersAsThresholds(parameterDefinition.id)
packages/apollo-react/src/canvas/components/Guardrails/centralized-guardrails-section.tsx:52
- When a translation or
labels.policyCaptionomits{{policyName}},splitreturns only the full template andafterisundefined, so the emphasized policy name is rendered nowhere even though the comment says it is appended. This can hide which governance policy is enforcing the section; preserve all token occurrences and append the name as a fallback when the token is absent.
const [before, after] = template.split('{{policyName}}');
packages/apollo-react/src/material/components/ap-chat/components/message/chat-message-content.tsx:38
getApolloMessageRenderersnow creates new inline component functions on every render. Because the selected function is rendered as<ApolloMessageRenderer>, any parent/context/message update changes its React type and remounts tool-call/tree content, resetting local state such as an expandedApToolCall. Keep these renderer component identities stable, for example by memoizing the list bylocaleat a hook-safe location or by moving the wrappers to module scope.
- Files reviewed: 112/113 changed files
- Comments generated: 0 new
- Review effort level: Lite
Both branches add the same README section, so make the two copies byte for byte identical and the merge takes one of them instead of conflicting on the wording. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🔵 Needs a closer look
Two moderate correctness issues remain in centralized guardrail handling, and the stale accessibility comment should be updated.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
packages/apollo-react/src/canvas/components/Guardrails/centralized-guardrail-utils.ts:276
- An empty threshold map (
{}ornull) should leave itskeySourcelist as a normal value row, but this presence check consumes that list andtoThresholdRowunconditionally imports the selected keys into an all-unset threshold table. This contradicts the comment above that only a map with a value should absorb the list; gate both consumption and key-source import on at least one numeric map entry.
packages/apollo-react/src/canvas/components/Guardrails/centralized-guardrail-utils.ts:46
- The key does not encode the origin type, so a valid BYO configuration named
builtincollides with the built-in row for the same validator and execution stage (...-builtin). Policies can contain both origins for one validator, so React will warn about duplicate keys and reuse one row's state instead of rendering both. Prefix the BYO portion (for example,byo:${guardrail.name ?? 'byo'}) before composing the key.
const origin = guardrail.isByo ? (guardrail.name ?? 'byo') : 'builtin';
packages/apollo-react/src/canvas/components/Guardrails/components/centralized-guardrail-row.tsx:123
- This comment describes the old disabled-button behavior, but the implementation immediately below intentionally returns a plain
<div>when there is noonSelect(and the story documents that behavior). The contradiction can mislead future changes about the accessibility contract; update the comment to explain that non-interactive rows are plain content and are not announced as unavailable.
// A row with nowhere to go is not a control. Both products render a disabled button there,
// which takes it out of the tab order anyway and announces it as unavailable rather than as
// the plain text it actually is.
- Files reviewed: 112/113 changed files
- Comments generated: 0 new
- Review effort level: Lite
…AL-574] The centralized section (#1161) needs the green both products already give the BYO origin chip, and had forked its own copy of the chip to get it. The tone belongs where the chip lives, so every consumer gets the same four. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
b4e90c4 to
0cde1e2
Compare
77aed7c to
d83f419
Compare
…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>
d83f419 to
4c97593
Compare
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The parameter resolver can silently drop malformed scalar values, and generated row keys can collide for valid wire strings.
Review effort: Lite
Findings: 1
Open (5)
The key is built by concatenating unescaped wire values, so distinct guardrails can collide: for…parameterTypeis explicitly unvalidated, but this predicate still trusts it. A BYO value such as… This name is a flex item beside the origin/status chips, buttruncatealone does not let a flex…valuesById.has()is true even when a threshold parameter is present withvalue: nullor… A BYO record with a missing/nullnameand a validator such aspii_detectionis currently given…
Resolved since last review (1)
4c97593 to
6dc84cc
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Two moderate issues remain in centralized-guardrail-utils.ts involving unsafe label lookup and collision-prone keys.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (6)
The key is built by concatenating unescaped wire values, so distinct guardrails can collide: for… Reject inherited or non-string option labels · NewparameterTypeis explicitly unvalidated, but this predicate still trusts it. A BYO value such as… This name is a flex item beside the origin/status chips, buttruncatealone does not let a flex…valuesById.has()is true even when a threshold parameter is present withvalue: nullor… A BYO record with a missing/nullnameand a validator such aspii_detectionis currently given…
6dc84cc to
eb1f061
Compare
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Three moderate findings remain unresolved in parameter resolution, option-label lookup, and host-specific callback typing.
Review effort: Lite
Findings: 1
Open (6)
The key is built by concatenating unescaped wire values, so distinct guardrails can collide: for… Reject inherited or non-string option labelsparameterTypeis explicitly unvalidated, but this predicate still trusts it. A BYO value such as… This name is a flex item beside the origin/status chips, buttruncatealone does not let a flex…valuesById.has()is true even when a threshold parameter is present withvalue: nullor… A BYO record with a missing/nullnameand a validator such aspii_detectionis currently given…
eb1f061 to
7ad101e
Compare
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Four moderate issues and one nit remain unresolved.
Review effort: Lite
Findings: 1
Open (1)
Resolved since last review (5)
The key is built by concatenating unescaped wire values, so distinct guardrails can collide: for… Reject inherited or non-string option labelsparameterTypeis explicitly unvalidated, but this predicate still trusts it. A BYO value such as…valuesById.has()is true even when a threshold parameter is present withvalue: nullor… A BYO record with a missing/nullnameand a validator such aspii_detectionis currently given…
7ad101e to
ac19463
Compare
…ils [AL-578] The read-only list of guardrails an organization's AI Trust Layer governance policy enforces on an agent, and the details content behind a row. Both products ship this today, each with its own copy; this is one component for both. - `CentralizedGuardrailsSection` renders the rows: name, origin chip, and for a broken BYO configuration `GuardrailList`'s status chip plus the sentence that says what to do about it. `onSelect` hands the selection back, because opening the details is host orchestration: Agents uses a dialog, Flow pushes a panel overlay. - `CentralizedGuardrailDetails` is that content, not a shell. The policy fields and the configuration render as a description list rather than the family's parameter editors in a read-only state: those are the MetadataForm stack, which has no read-only mode, and a disabled input cannot be focused, so its value is not selectable, not copyable and skipped by a screen reader. - `centralized-types.ts` mirrors both products' policy schemas structurally, so a host passes its own zod-inferred types with no mapping. A governance record is not a variant of `GuardrailBuilderValue`: no `id`, `scopes` at the top level, `action` a bare discriminator. - `resolveCentralizedGuardrailParameters` lifts a built-in's `entities` and `entityThresholds` onto the parameter shape, so one resolver covers both origins, and labels them from the matching definition when there is one. - `definitions` is optional and `undefined` means "not loaded yet", which is what keeps a row from claiming a configuration was deleted while the catalog is still in flight. - The row's accessible name is its own text. Both products put an `aria-label` there, which overrides the content and hides the description, the provider and the remediation message from screen readers. - `guardrails.centralized.*` ids in one builder function, English harvested from both products where they agree. The five divergences are declared with a reason in `centralized-parity.test.ts`. Scope, action, type and description labels reuse the builder's ids, and the provider, BYO and status chip labels the list's, rather than twinning them. - The details' note passes `mt-0`: wind's `AlertDescription` is unconditionally `mt-1`, which assumes an `AlertTitle` above it, so a title-less alert renders its text below its icon. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ac19463 to
ae6e1fb
Compare


Implements AL-578: the read-only section for the
guardrails an organization's AI Trust Layer governance policy enforces on an agent, and the details
content behind a row. Both products ship this today, each with its own copy and its own component
tree.
What lands
centralized-types.tsCentralizedGuardrailand the five shapes around itcentralized-guardrail-utils.tsresolveCentralizedGuardrailParameters, three label formatters. Pure, React-freecentralized-guardrails-section.tsxCentralizedGuardrailsSectioncentralized-guardrail-details.tsxCentralizedGuardrailDetailsi18n.tsCentralizedGuardrailsLabels,CENTRALIZED_GUARDRAILS_EN_LABELS,CENTRALIZED_GUARDRAILS_EN_MESSAGES,resolveCentralizedGuardrailsLabels,useCentralizedGuardrailsLabelsPlus four private components (row, origin chip, parameters, detail field), a README section,
Components/UiPath/Centralized Guardrailsstories including both host shells, and theguardrails.centralized.*catalog ids.Decisions worth checking
GuardrailBuilderValue. Noid,scopesat the top level rather than under aselector,actiona bare discriminator.centralized-types.tsmirrors both products' zod schemas structurally, so each host passes itsown inferred type with no mapping:
executionStagestaysstringbecause both parse it as one,and
actionis the closed four-value union both close it to, which a TypeScript string enummember assigns to, so Agents'
ActionTypefits.their own (
useAiTrustLayerGovernancePolicy,useGovernance,GuardrailDefinitionsContext), soreading one here would tie the component to whichever host it was written in.
getApplicableCentralizedGuardrailsis theagent-kind predicate both products already run, exported so neither rewrites it. An empty list
renders nothing, which is what both do;
emptyStateoverrides that.definitionsis optional andundefinedmeans "not loaded yet", which is what keeps a rowfrom claiming a configuration was deleted while the catalog is still in flight. An empty array
means it loaded and the configuration really is gone.
connector parameters and a built-in as
entities/entityThresholds.resolveCentralizedGuardrailParameterslifts the built-in fields onto the parameter shape so onepath covers both, and it discriminates on each value's own shape rather than on
parameterType,which is an unvalidated wire string.
MetadataForm stack and have no read-only mode, and these values arrive as untyped wire data
rather than as
GuardrailValidatorParameters. A disabled input also cannot be focused, so itscontent is not selectable, not copyable and skipped by a screen reader.
aria-label.mt-0on the details' title-less note, the workaroundGuardrailStatusBanneralreadycarries. Wind's
AlertDescriptionis unconditionallymt-1, which assumes anAlertTitleabove it, while
alertVariantspins the icon attop-3.5, so a title-less alert renders itstext 4px below its icon. The real fix is one line in
alertVariants.QA-visible changes
Both products
of
USSocialSecurityNumber, matching the guardrail editor next to it."Severity thresholds" for harmful content, "Detection thresholds" for PII) instead of one generic
pair, which stays as the fallback when no definition matched.
parameterType: a scalar taggedmap-enumshows instead of vanishing, and a threshold map persisted asnullleaves itsentity list as its own labelled row instead of an empty table.
nameshows its validator id rather than the built-in's curated name.GuardrailList's status chip (Unavailable/Disabled)next to the name. The remediation sentence both products already show stays underneath: the
chip is for finding the row, the sentence for fixing it.
aria-label="View details for X"today, which overrides the content and hides the description,the provider, the scopes and the broken-configuration message from screen readers.
Agents (
frontend-sw)dialog itself stays Agents', since the component is content, not a shell.
unstyled+hideHeaderkeep the existingSectionAccordionchrome.infoandmanagedMessagetake Flow's wording,docsLinkbecomes "View centralizedguardrails documentation".
Flow (
flow-workbench)descriptionreuses the builder's "Guardrail description" label;thresholdsFallbackkeepsFlow's plural wording.
validator === 'harmful_content'threshold-label branch is replaced by the definition-drivenlabel.
Copy and localization
English harvested from both products, in one builder function with the rest of the family. Where
the two agree this package says the same thing; the five places they disagree are declared with
a reason in
centralized-parity.test.tsand asserted against both transcriptions, so the suitefails on an undeclared difference, a stale declaration, or a third wording we invented.
Scope, action, type and description labels reuse the builder's ids, and the provider, BYO and
status chip labels
GuardrailList's, rather than declaringguardrails.centralized.*twins, sothey cannot drift from the strings next to them (the builder's are already translated in all
thirteen locales). English only for the ids this block declares itself, like the rest of the
package:
chore(l10n): sync from Localizationowns the other catalogs.useSafeLinguirenders the English default until it runs, so nothing is blank.Verification
tscclean,biomeclean on the changed files.Guardrailsdirectory: 31 files, 640 testspassing, with jest-axe on both rendering suites.
Review questions
remediation sentence. It is
GuardrailList's chip, so the two sections mark a brokenconfiguration the same way; if you would rather the row stayed at parity, removing it is one
&&.from drifting, but it does couple the label blocks.
findCentralizedBuiltInDefinitionis new. Neither product looks a built-in definition uptoday; it exists only so the configuration rows can be labelled and the entities named. It never
feeds the display name or description, because a policy can enforce a validator the tenant has
no definition for.
🤖 Generated with Claude Code