refactor(plugin-grid): derive the row-menu predicate declarations from the spec-owned authoring type (#4429)

.changeset/grid-row-menu-predicate-derive-4429.md

@@ -0,0 +1,13 @@
+---
+'@object-ui/plugin-grid': minor
+---
+
+grid row menu — the built-in Edit/Delete predicate declarations are derived from the spec-owned authoring type, not hand-restated
+
+`packages/plugin-grid/src/components/RowActionMenu.tsx` carried its own `BuiltinRowActionPredicates` interface (`{ visibleWhen?: unknown; disabledWhen?: unknown }`) and read it at six declaration sites: both `RowActionMenuProps` predicate props, the shared `isBuiltinRowActionVisible` gate, both `planRowActionMenu` parameters, and the `BuiltinRowActionItem` component. Nothing tied any of them to the type whose values they receive, so a rename at the source would have left every one compiling against a shape that no longer existed — the objectui#3009 hand-copy family, and the mirror of what PR #4423 collapsed in the data-table.
+
+**Measured true source.** These predicates do NOT flow from `DataTableSchema.rowEditPredicates` / `rowDeletePredicates` — this surface is never handed those keys. `ObjectGrid` resolves the object's `userActions.edit` / `delete` through `resolveRowCrudAffordances`, which returns `CrudAffordances['editPredicates']` / `['deletePredicates']`: the spec-owned `RowCrudPredicates` (ADR-0103, `@objectstack/spec/data`), parsed in exactly one place and re-exported by `@object-ui/core`. Each site now derives from that — per-key `Pick` for the planner (visibility is all it decides), one union alias for the two consumers that serve both built-ins. Measured: with `visibleWhen` renamed at the source, the previous hand-written declarations produce ZERO diagnostics in this package while the derived ones fail to compile at the declarations themselves.
+
+**Graded `minor` rather than `patch`** because a published type narrows (the objectui#4403 criterion). `RowActionMenuProps.editPredicates` / `deletePredicates` move from `unknown`-valued keys to the spec's `Expression | ExpressionInput` — the authored CEL shorthand or its `{ dialect, source }` envelope — so a consumer passing an `unknown`-typed value, or a bare boolean, stops type-checking. No runtime consumer breaks and no behavior changes; `@object-ui/core` retired the same `unknown` imprecision at its own seam, and this was the last copy of it. (PR #4423's data-table twin stayed `patch` because `DataTableSchema`'s keys were already declared `unknown` — deriving there narrowed nothing.)
+
+No runtime code was changed, and the package's suite passes unchanged. Alongside it, the "a disabled item still counts toward the menu" rule gains the pin it never had where a user meets it: a row whose only action is `disabledWhen`-gated keeps its "⋮" trigger, and that trigger opens the item, present and `aria-disabled`. The two halves of that rule live in different functions, and each half's own test stayed green while the other regressed. The planner-level case that claimed to pin this is renamed to the verdict it actually decides — the planner never reads `disabledWhen`, so its fixture behaved identically to `{}`.

packages/plugin-grid/src/components/RowActionMenu.tsx

@@ -15,7 +15,7 @@ import {
   DropdownMenuTrigger,
 } from '@object-ui/components';
 import { Edit, Trash2, MoreVertical } from 'lucide-react';
-import { evalRowPredicate } from '@object-ui/core';
+import { evalRowPredicate, type CrudAffordances } from '@object-ui/core';
 import { useObjectTranslation, useRowPredicate, useCapabilityGate, usePredicateScope } from '@object-ui/react';
 
 const ROW_ACTION_FALLBACKS: Record<string, string> = {
@@ -54,16 +54,47 @@ export interface RowActionDef {
   [key: string]: any;
 }
 
-/** Per-record CEL predicates gating a built-in Edit/Delete row action
- * (objectui#2614). Bare CEL string or `{ dialect, source }` envelope,
- * evaluated per row via `useRowPredicate` — same machinery as custom
- * actions' `visible` / `disabled`. */
-export interface BuiltinRowActionPredicates {
-  /** Evaluates false → the item is not rendered for this row. Fail-closed. */
-  visibleWhen?: unknown;
-  /** Evaluates true → the item renders disabled for this row. Fail-soft. */
-  disabledWhen?: unknown;
-}
+/**
+ * Per-record CEL predicates gating a built-in Edit/Delete row action
+ * (objectui#2614) — DERIVED from the type whose values this file actually
+ * receives, never restated by hand (objectui#4429, inheriting PR #4423).
+ *
+ * The source is NOT the data-table's `DataTableSchema.rowEditPredicates` /
+ * `rowDeletePredicates`: this surface is never handed those keys. `ObjectGrid`
+ * resolves the OBJECT's `userActions.edit` / `delete` through
+ * `resolveRowCrudAffordances` (`../rowCrudAffordances`), which returns
+ * `CrudAffordances['editPredicates']` / `['deletePredicates']` — the spec-owned
+ * `RowCrudPredicates` (ADR-0103), parsed in exactly one place
+ * (`@object-ui/core`'s `normalizeUserAction`) and re-exported there from
+ * `@objectstack/spec/data`. Deriving from the data-table's twins would have
+ * bound this file to a schema key it never sees: a NEW wrong coupling in place
+ * of the old missing one.
+ *
+ * The union of the two twins is deliberate, exactly as in PR #4423: every
+ * consumer that reads this alias serves BOTH built-ins (`name: 'edit' |
+ * 'delete'`), so it may only read keys that BOTH affordance keys declare — if
+ * either twin dropped one, the read stops compiling instead of silently reading
+ * `undefined`. Today the twins are declared as one named type, so the union
+ * collapses onto it; the construction is what states the constraint that
+ * outlives that coincidence.
+ *
+ * Deriving also TIGHTENS what this file accepts, and that is the point rather
+ * than a side effect: the hand-written pair typed both keys `unknown`, while
+ * the spec types them `Expression | ExpressionInput` — the authored CEL
+ * shorthand or its `{ dialect, source }` envelope. `@object-ui/core` retired
+ * the same imprecision at its own seam and said so in as many words ("The local
+ * `unknown` was imprecision, not a deliberate dialect"); this was the last copy
+ * of it.
+ *
+ * Behavior is unchanged and stays the posture `RowCrudActionOverrideSchema`
+ * specifies: `visibleWhen` fails CLOSED (the item is not rendered for this row),
+ * `disabledWhen` fails soft (the item renders disabled), both evaluated per row
+ * on the canonical CEL engine — the same machinery as custom actions'
+ * `visible` / `disabled`.
+ */
+export type BuiltinRowActionPredicates = NonNullable<
+  CrudAffordances['editPredicates'] | CrudAffordances['deletePredicates']
+>;
 
 export interface RowActionMenuProps {
   /** The row data record */
@@ -166,9 +197,14 @@ function evalRowActionVisibility(
  * `visibleWhen` counts as a declared gate by `!= null`, not by truthiness —
  * `visibleWhen: false` hides the item rather than reading as "ungated". This is
  * the item's historical rule, extracted verbatim.
+ *
+ * The parameter is consumption-shaped — visibility is all this decides, so
+ * `disabledWhen` is deliberately absent — but `Pick`ed from the derived
+ * authoring shape rather than hand-written, so it is a SUBSET of that shape by
+ * construction (objectui#4429).
  */
 export function isBuiltinRowActionVisible(
-  predicates: BuiltinRowActionPredicates | undefined,
+  predicates: Pick<BuiltinRowActionPredicates, 'visibleWhen'> | undefined,
   name: 'edit' | 'delete',
   row: any,
   scope: Record<string, unknown>,
@@ -253,7 +289,11 @@ export interface RowActionMenuPlan {
  * `_actions` column's `cell`, so the decision lives here.
  *
  * Only visibility is decided here — a `disabled` item still renders (greyed
- * out) and still COUNTS, exactly as before. Capability filtering
+ * out) and still COUNTS, exactly as before. That counting is pinned where it
+ * can be OBSERVED, on the rendered menu, by `RowActionMenu.emptyGuard.test.tsx`'s
+ * "keeps the trigger for a row whose only item renders merely DISABLED"
+ * (objectui#4429) — this function never reads `disabledWhen`, so no assertion on
+ * its return value can pin that claim. Capability filtering
  * (ADR-0066 D4 / framework#3923) happens upstream of this call, on the declared
  * set, so its verdict reaches the guard the same way.
  *
@@ -290,8 +330,17 @@ export function planRowActionMenu(input: {
   canDelete?: boolean;
   onEdit?: unknown;
   onDelete?: unknown;
-  editPredicates?: BuiltinRowActionPredicates;
-  deletePredicates?: BuiltinRowActionPredicates;
+  /**
+   * The predicate parameters are consumption-shaped — this function reads
+   * `visibleWhen` and nothing else, and the signatures keep saying so — but each
+   * is `Pick`ed from the exact affordance key its production caller passes
+   * (`ObjectGrid` → `resolveRowCrudAffordances`), rather than hand-restated
+   * (objectui#4429). Same subset as before, minus the drift: a rename in the
+   * spec-owned `RowCrudPredicates` fails here at compile time instead of leaving
+   * a stale hand-copy that still type-checks against nothing.
+   */
+  editPredicates?: Pick<NonNullable<CrudAffordances['editPredicates']>, 'visibleWhen'>;
+  deletePredicates?: Pick<NonNullable<CrudAffordances['deletePredicates']>, 'visibleWhen'>;
   objectFields?: unknown;
 }): RowActionMenuPlan {
   const { row, scope, objectFields } = input;
@@ -384,6 +433,13 @@ const RowActionMenuItem: React.FC<{
  */
 export const BuiltinRowActionItem: React.FC<{
   name: 'edit' | 'delete';
+  /**
+   * The whole authoring shape, because this component reads BOTH keys —
+   * derived from the affordance type rather than hand-copied (objectui#4429).
+   * This is the site with the most to gain: a hand-copy that keeps one key in
+   * common with a renamed source satisfies TS weak-type detection and drifts
+   * with ZERO diagnostics (measured on the data-table twin in PR #4423).
+   */
   predicates?: BuiltinRowActionPredicates;
   row: any;
   icon: React.ReactNode;

packages/plugin-grid/src/components/__tests__/RowActionMenu.emptyGuard.test.tsx

@@ -31,18 +31,21 @@
  * `rowActions` that carry no predicate.
  */
 import { describe, it, expect, vi, afterEach } from 'vitest';
-import { render, screen, waitFor, cleanup } from '@testing-library/react';
+import { render, screen, waitFor, cleanup, fireEvent } from '@testing-library/react';
 import '@testing-library/jest-dom';
 import React from 'react';
 import { PredicateScopeProvider, ActionProvider, SchemaRendererProvider } from '@object-ui/react';
 import { registerAllFields } from '@object-ui/fields';
+import type { CrudAffordances } from '@object-ui/core';
 import { RowActionMenu, planRowActionMenu } from '../RowActionMenu';
 import { ObjectGrid } from '../../ObjectGrid';
 
 registerAllFields();
 
 /** The real `userActions.edit.visibleWhen` shape (objectui#2614). */
 const NOT_FROZEN = 'record.frozen != true';
+/** Its `disabledWhen` counterpart — HOLDS for the frozen row, so it greys out. */
+const IS_FROZEN = 'record.frozen == true';
 /** The reporter's shape: an action gated for a role the viewer is not in. */
 const IS_APPROVER = 'record.approver == "u-me"';
 
@@ -136,6 +139,44 @@ describe('RowActionMenu — the "⋮" counts renderable items, not handlers (#35
     renderMenu({ rowActions: ['send_email'], onAction: () => {} });
     expect(trigger()).toBeInTheDocument();
   });
+
+  /**
+   * The other side of #3562's rule, observed where a user meets it: a row whose
+   * only item is greyed out is NOT an empty menu. `disabledWhen` is a rendering
+   * verdict, not a visibility one, so the item counts, the trigger survives, and
+   * what it opens is that item — present and `aria-disabled` (objectui#4429).
+   *
+   * End-to-end on purpose: the two halves live apart — the count in
+   * `planRowActionMenu`, the greying in `BuiltinRowActionItem` — and each half's
+   * own test stays green while the other regresses. Neither could observe this
+   * pairing before: the planner's cases read its return value (and the planner
+   * never reads `disabledWhen` at all — see the renamed case below), while the
+   * item's own cases (`RowActionMenu.test.tsx`) render `BuiltinRowActionItem`
+   * directly inside an already-open dropdown, never through the component that
+   * decides whether a trigger exists at all.
+   *
+   * This is the level where the two halves actually meet: `RowActionMenu` is
+   * both the caller of the planner and the renderer of the item. The
+   * `ObjectGrid` describe further down is a level ABOVE it (column alignment
+   * plus the async `userActions` plumbing) and its cases read `visibleWhen`
+   * only, so it did not pin this either.
+   */
+  it('keeps the trigger for a row whose only item renders merely DISABLED', async () => {
+    renderMenu({
+      canEdit: true,
+      onEdit: () => {},
+      editPredicates: { disabledWhen: IS_FROZEN },
+    });
+
+    // Counted: a greyed-out item is still an item, so the row keeps its "⋮".
+    expect(trigger()).toBeInTheDocument();
+
+    // Radix opens on `pointerdown` (a plain click does nothing) and portals its
+    // content on the next tick, hence the async find.
+    fireEvent.pointerDown(trigger()!, { button: 0, ctrlKey: false, pointerType: 'mouse' });
+    const item = await screen.findByTestId('row-action-builtin-edit');
+    expect(item).toHaveAttribute('aria-disabled', 'true');
+  });
 });
 
 /**
@@ -446,13 +487,33 @@ describe('planRowActionMenu', () => {
       .toMatchObject({ legacy: ['send_email'], menuCount: 1 });
   });
 
-  it('still counts an item that renders merely DISABLED', () => {
+  // Renamed from "still counts an item that renders merely DISABLED"
+  // (objectui#4429). The planner never reads `disabledWhen`, so this fixture
+  // behaves identically to `{}` and could not pin the disabled-counting claim
+  // its old name made — that claim now lives where it can actually be observed,
+  // in "keeps the trigger for a row whose only item renders merely DISABLED"
+  // above. What survives here is the verdict this case really does decide, and
+  // it is worth deciding: a predicate object is not itself a gate.
+  it('an object with no `visibleWhen` does not hide the item', () => {
+    // Typed as what the production caller actually hands the planner —
+    // `resolveRowCrudAffordances`' `editPredicates`, whose spec-owned shape
+    // (`RowCrudPredicates`, ADR-0103) carries `disabledWhen` alongside
+    // `visibleWhen`. The planner's own parameter is now `Pick`ed from that same
+    // key rather than hand-restated (objectui#4429), so it stays the deliberate
+    // `visibleWhen`-only subset: a bare object literal trips excess-property
+    // checking, while the identical value reaches it unremarked in production.
+    // Annotating from the authoring type keeps the fixture honest about which
+    // key is being ignored, rather than deleting the key and quietly renaming
+    // the case.
+    const editPredicates: CrudAffordances['editPredicates'] = {
+      disabledWhen: IS_FROZEN,
+    };
     expect(planRowActionMenu({
       ...base,
       row: FROZEN,
       canEdit: true,
       onEdit: noop,
-      editPredicates: { disabledWhen: 'record.frozen == true' },
+      editPredicates,
     })).toMatchObject({ edit: true, menuCount: 1 });
   });