fix(predicate): 「空谓词」不再算已声明门 —— 唯一定义下沉 core,渲染面 / SchemaRenderer / 执行门同读 (#3850, #3862)

.changeset/empty-predicate-declared-gate-3850-3862.md

@@ -0,0 +1,74 @@
+---
+"@object-ui/core": patch
+"@object-ui/components": patch
+"@object-ui/react": patch
+---
+
+An empty predicate is no longer a declared gate anywhere (objectui#3850, objectui#3862)
+
+"Is a gate DECLARED on this key — is there a condition to reach a verdict on?" was
+answered three times in this repo, with three different scopes, and the widest
+answers sat on `disabled`, where the mistake is not benign:
+
+- `hasDeclaredVisibilityGate` (the action face) asked `!= null && !== ''`, so every
+  OBJECT counted — including `{ dialect: 'cel', source: '' }`. That envelope is not
+  a hand-written curiosity: `@objectstack/spec`'s `ExpressionInputSchema` normalizes
+  every authored predicate into one, so "the author left the predicate empty"
+  compiles to exactly it. The verdict path normalized the same value back to
+  `undefined`, and `evaluateCondition(undefined)` answers `true` — "no condition, so
+  visible/enabled". On `visible` that `true` means SHOW, so the two mistakes
+  cancelled; on `disabled` it means GREY, so they compounded: a button disabled
+  forever that no author asked to disable (objectui#3850, the residue objectui#3842
+  left behind).
+- `SchemaRenderer` asked `disabled !== undefined` inline, one notch wider again, so
+  `disabled: null` greyed out too — on the GENERIC rendering path, since that block
+  runs for every node type, and not as an internal flag either: `_disabled` is
+  forwarded to the component as a real `disabled` prop (objectui#3862).
+- `ActionRunner`'s execution gates asked "does this normalize to something
+  evaluable?" — the scope that turned out to be right (objectui#3848 / objectui#3872).
+
+There is now ONE definition, `hasDeclaredPredicate`, exported from
+`@object-ui/core` (`evaluator/declaredPredicate.ts`, beside the `toPredicateInput`
+normalizer it is derived from): a gate is declared when normalization still leaves a
+condition to evaluate. `''`, a whitespace-only string, an empty-`source` envelope
+and any non-predicate value (`0`, `{}`) are NOT declared; `false` IS (a verdict is
+not a missing gate — objectui#3812). `hasDeclaredVisibilityGate` keeps its name as a
+re-export of it, so the five member-action renderer call sites, `DeclaredActionsBar`
+and `record-quick-actions` are unchanged and inherit the scope;
+`SchemaRenderer`'s `disabled` / `disabledOn` chain and `ActionRunner`'s two gates
+read the same function. No consumer got a local "and also check for empty" test —
+that fourth dialect is what objectui#3842 / objectui#3849 spent two PRs merging away.
+
+Measured behaviour change, `action:button` and the generic path, before → after:
+
+| value | `visible` | `disabled` | `enabled` | `SchemaRenderer` `disabled` prop |
+|---|---|---|---|---|
+| `''` | shown → shown | on → on | on → on | forwarded → absent |
+| `null` | shown → shown | on → on | on → on | forwarded → absent |
+| `{ dialect: 'cel', source: '' }` | shown → shown | GREY → on | on → on | forwarded → absent |
+| `{ source: '' }` | shown → shown | GREY → on | on → on | forwarded → absent |
+| `'   '` (whitespace) | HIDDEN → shown | on → on | GREY → on | forwarded → absent |
+| `0` / `{}` (not predicates) | shown → shown | GREY → on | on → on | forwarded → absent |
+| `true` / `false` / bare CEL / `${…}` / non-empty envelope | unchanged | unchanged | unchanged | unchanged |
+
+Every row moves toward "there is no gate here", never away from it, and no value
+that HAS a verdict changes it — the verdict is still read from the raw value, only
+the gate in front of it narrowed. Two rows are behaviour changes rather than the
+equivalence the ruling expected, and are pinned as such: the whitespace string moves
+on `visible` / `enabled` (it used to normalize to `'${   }'`, which evaluates falsy,
+so a predicate that says nothing HID the action from everyone), and non-predicate
+junk stops greying controls out (fail-open, the posture `ActionRunner` already
+committed to).
+
+One blank spelling is knowingly still outside the scope: an envelope whose `source`
+is blank but not EMPTY (`{ dialect: 'cel', source: '   ' }`) — the normalizer folds a
+`source` of `''` and does not trim, so the string spelling of a blank predicate is
+trimmed and the envelope spelling is not, and `disabled` still greys out for that one
+value. The ruling enumerated three empty spellings; this is a fourth, measured and
+filed as objectui#3960 rather than widened in here.
+
+One chain is deliberately untouched: `SchemaRenderer`'s `visible` / `visibleWhen` /
+`visibleOn` / `visibility` / `hidden` / `hiddenOn` legs keep `!== undefined`, because
+narrowing them would change ALIAS PRECEDENCE, not just emptiness. The `hidden` legs
+are not negated and therefore carry this same defect with the polarity that makes the
+node vanish — measured, out of this ruling's scope, filed as objectui#3955.

packages/components/src/index.ts

@@ -42,9 +42,12 @@ export { cva } from 'class-variance-authority';
 export { getLazyIcon, isLucideIconName, LazyIcon, toKebabIconName } from './lib/lazy-icon';
 
 // The member-action visibility gate — "did this action DECLARE a `visible` gate
-// at all?" (`!= null && !== ''`), the single definition objectui#3492
-// established and PR #3816 / #3825 / #3836 applied to every member-action gate
-// in this package and in `plugin-grid`.
+// at all?", the single definition objectui#3492 established and PR #3816 /
+// #3825 / #3836 applied to every member-action gate in this package and in
+// `plugin-grid`. Since objectui#3850 the answer is "normalization still leaves a
+// condition to evaluate" (so an empty-`source` envelope is NOT a gate, where the
+// older `!= null && !== ''` counted every object), and this name is a re-export
+// of core's one definition, `hasDeclaredPredicate`.
 //
 // Exported because the family has a member OUTSIDE these packages: app-shell's
 // `DeclaredActionsBar` mounts an object's server-declared actions as plain JSX

packages/components/src/renderers/action/__tests__/action-empty-predicate-scope.test.tsx

@@ -0,0 +1,257 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * objectui#3850 — how wide "empty predicate" is on the action face, now that
+ * `hasDeclaredVisibilityGate` is a re-export of core's one definition
+ * (`hasDeclaredPredicate`, `evaluator/declaredPredicate.ts`) instead of a
+ * second, narrower answer to the same question.
+ *
+ * The residue objectui#3842 left behind: `hasDeclaredVisibilityGate` asked
+ * `!= null && !== ''`, so every OBJECT counted as a declared gate — including
+ * `{ dialect: 'cel', source: '' }`, the shape `@objectstack/spec`'s
+ * `ExpressionInputSchema` produces when an author leaves a predicate empty, and
+ * therefore the empty shape most likely to reach a renderer from real metadata.
+ * The verdict path normalized the same value to `undefined`, and
+ * `evaluateCondition(undefined)` answers `true` = "no condition → visible /
+ * enabled". On `visible` that `true` means SHOW, so the two mistakes cancelled;
+ * on `disabled` it means GREY, so they compounded — a button disabled forever
+ * that no author asked to disable.
+ *
+ * ## What each case detects
+ *
+ *   • the `disabled` leg's two envelope cases (`{ dialect, source: '' }` and
+ *     `{ source: '' }`) → clickable. THE defect, one case per row of the #3850
+ *     table's "残留" lines. Restore `!= null && !== ''` in the definition and
+ *     exactly these go red.
+ *   • the `disabled` leg's junk cases (`0`, `{}`) → clickable. Behaviour change
+ *     in the same fail-open direction: a value the evaluator cannot read must not
+ *     be the reason a control is dead. (`ActionRunner` already committed this
+ *     module family to `catch { isDisabled = false }`.)
+ *   • `disabled: true` / a truthy expression / a truthy CEL envelope → still
+ *     greyed. Anti-mutation guards: "never disable anything" satisfies most of
+ *     this file on its own, and these refuse it.
+ *   • `disabled: false` and no `disabled` at all → still clickable, so widening
+ *     "empty" did not swallow a declared-and-false verdict (objectui#3812).
+ *   • the `visible` leg's empty cases → still rendered, which is the EQUIVALENCE
+ *     the objectui#3850 ruling asked for on that family. Two of the three rows
+ *     are equivalence in the strict sense (`''` and the envelope reached "shown"
+ *     before this change too, by cancellation); the whitespace row is NOT — see
+ *     the next block, which pins the change rather than hiding it.
+ *
+ * ## The whitespace row moves, and this suite says so out loud
+ *
+ * Measured on this tree, `action:button` before → after:
+ *
+ *   | value                          | `visible`      | `disabled`  | `enabled`  |
+ *   |--------------------------------|----------------|-------------|------------|
+ *   | `''`                           | shown → shown  | on → on     | on → on    |
+ *   | `{ dialect:'cel', source:'' }` | shown → shown  | GREY → on   | on → on    |
+ *   | `{ source: '' }`               | shown → shown  | GREY → on   | on → on    |
+ *   | `'   '` (whitespace only)      | HIDDEN → shown | on → on     | GREY → on  |
+ *   | `0` / `{}`                     | shown → shown  | GREY → on   | on → on    |
+ *
+ * `'   '` used to be a declared gate whose verdict came from `'${   }'` (the
+ * normalizer wraps a whitespace string rather than folding it), which evaluates
+ * falsy — so a predicate that says nothing HID the action from everyone, and
+ * greyed it out through the negated `enabled` leg. Moving it to "no gate" is the
+ * direction this gate's own doc has always claimed ("an empty predicate must not
+ * hide the action from everyone either"); it is a behaviour CHANGE on `visible`
+ * and `enabled` all the same, so it is pinned as one.
+ *
+ * ## Why one leaf plus an identity assertion covers five call sites
+ *
+ * `action:button`, `action:icon`, `action:group`'s inline button and dropdown
+ * item, and `action:menu`'s item all import the same symbol from
+ * `../visibility-gate` — unchanged by this move, which is the point of keeping
+ * the name. The identity case below asserts that symbol IS core's
+ * `hasDeclaredPredicate`, so the scope pinned here is the scope all five read; a
+ * leaf that re-spelled the question locally would break that claim, not hide
+ * behind it. The `''` rows for the member leaves stay where objectui#3842 /
+ * objectui#3849 put them (`action-member-disabled-declared-gate.test.tsx`).
+ */
+
+import { describe, it, expect } from 'vitest';
+import { render, screen } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import React from 'react';
+import { ComponentRegistry, hasDeclaredPredicate } from '@object-ui/core';
+import { PredicateScopeProvider } from '@object-ui/react';
+// Module-scope side-effect import so the renderer is in the registry when
+// `ComponentRegistry.get` runs (the light `dom` project does not load the
+// `@object-ui/components` graph), per AGENTS.md §测试纪律.
+import '../action-button';
+import { hasDeclaredVisibilityGate } from '../visibility-gate';
+
+/** Mount the leaf the way `action:bar` mounts it: whole action spread onto `schema`. */
+function renderLeaf(action: Record<string, unknown>, scope: Record<string, unknown> = {}) {
+  const Renderer = ComponentRegistry.get('action:button');
+  if (!Renderer) throw new Error('action:button is not registered');
+  return render(
+    <PredicateScopeProvider scope={scope}>
+      <Renderer schema={{ ...action, type: 'action:button', actionType: 'script' }} />
+    </PredicateScopeProvider>,
+  );
+}
+
+const ACT = { name: 'act', label: 'Act', type: 'script' };
+const act = () => screen.getByRole('button', { name: 'Act' });
+
+/** The shapes with nothing to evaluate, by the spelling that produces them. */
+const EMPTY_SHAPES: Array<{ label: string; value: unknown }> = [
+  { label: "'' (empty string)", value: '' },
+  { label: "'   ' (whitespace only)", value: '   ' },
+  { label: "{ dialect: 'cel', source: '' } (what `objectstack build` emits)", value: { dialect: 'cel', source: '' } },
+  { label: "{ source: '' } (envelope without a dialect)", value: { source: '' } },
+  { label: 'null', value: null },
+];
+
+/** Values the evaluator cannot read at all. */
+const JUNK_SHAPES: Array<{ label: string; value: unknown }> = [
+  { label: '0', value: 0 },
+  { label: '{} (an object with no source)', value: {} },
+];
+
+describe('the one definition — `hasDeclaredVisibilityGate` is core\'s `hasDeclaredPredicate` (objectui#3850)', () => {
+  it('is the same function object, not a same-shaped copy', () => {
+    // A re-spelled twin here is how this question grew three scopes in the first
+    // place (objectui#3142 is what copies of one answer cost). Identity, so the
+    // five member-action call sites and this suite cannot drift apart.
+    expect(hasDeclaredVisibilityGate).toBe(hasDeclaredPredicate);
+  });
+
+  it('answers the empty spellings and the junk with "no gate", and a declared boolean with "gate"', () => {
+    for (const { label, value } of [...EMPTY_SHAPES, ...JUNK_SHAPES]) {
+      expect(hasDeclaredVisibilityGate(value), `${label} must not count as a declared gate`).toBe(false);
+    }
+    expect(hasDeclaredVisibilityGate(false)).toBe(true);
+    expect(hasDeclaredVisibilityGate(true)).toBe(true);
+  });
+});
+
+describe('action:button `disabled` — an empty predicate is not a gate (objectui#3850)', () => {
+  it.each(EMPTY_SHAPES)('disabled: $label → the button stays clickable', ({ value }) => {
+    renderLeaf({ ...ACT, disabled: value });
+    expect(act()).not.toBeDisabled();
+  });
+
+  it.each(JUNK_SHAPES)('disabled: $label (not a predicate) → the button stays clickable', ({ value }) => {
+    renderLeaf({ ...ACT, disabled: value });
+    expect(act()).not.toBeDisabled();
+  });
+
+  it('disabled: true → still greyed out', () => {
+    renderLeaf({ ...ACT, disabled: true });
+    expect(act()).toBeDisabled();
+  });
+
+  it('disabled: false → still clickable (a verdict, not a missing gate)', () => {
+    renderLeaf({ ...ACT, disabled: false });
+    expect(act()).not.toBeDisabled();
+  });
+
+  it('no `disabled` at all → still clickable', () => {
+    renderLeaf({ ...ACT });
+    expect(act()).not.toBeDisabled();
+  });
+
+  it("disabled: { dialect: 'cel', source: 'true' } → still greyed out (a NON-empty envelope is a gate)", () => {
+    renderLeaf({ ...ACT, disabled: { dialect: 'cel', source: 'true' } });
+    expect(act()).toBeDisabled();
+  });
+
+  it('an expression-valued `disabled` keeps its verdict, both ways', () => {
+    const gated = { ...ACT, disabled: 'features.locked == true' };
+    const { unmount } = renderLeaf(gated, { features: { locked: true } });
+    expect(act()).toBeDisabled();
+    unmount();
+    renderLeaf(gated, { features: { locked: false } });
+    expect(act()).not.toBeDisabled();
+  });
+});
+
+describe('action:button `visible` — the equivalence the ruling asked for, and the one row that moves', () => {
+  it.each(EMPTY_SHAPES)('visible: $label → the action is still rendered', ({ value }) => {
+    renderLeaf({ ...ACT, visible: value });
+    expect(act()).toBeInTheDocument();
+  });
+
+  it.each(JUNK_SHAPES)('visible: $label (not a predicate) → still rendered', ({ value }) => {
+    renderLeaf({ ...ACT, visible: value });
+    expect(act()).toBeInTheDocument();
+  });
+
+  it('visible: false → still hidden, and visible: true / a true predicate still shown', () => {
+    const { container, unmount } = renderLeaf({ ...ACT, visible: false });
+    expect(container.querySelector('button')).toBeNull();
+    unmount();
+    renderLeaf({ ...ACT, visible: true });
+    expect(act()).toBeInTheDocument();
+  });
+
+  it('a false expression still hides it (the gate narrowed; evaluation did not change)', () => {
+    const { container } = renderLeaf({ ...ACT, visible: 'features.beta == true' }, { features: { beta: false } });
+    expect(container.querySelector('button')).toBeNull();
+  });
+
+  it("CHANGED: visible: '   ' was HIDDEN before this ruling and is now shown", () => {
+    // Not equivalence — the honest row. `'   '` normalized to `'${   }'`, which
+    // evaluates falsy, so a predicate that says nothing hid the action from
+    // everyone. See this file's header table.
+    renderLeaf({ ...ACT, visible: '   ' });
+    expect(act()).toBeInTheDocument();
+  });
+});
+
+describe('action:button legacy `enabled` leg — same definition, same scope', () => {
+  it.each(EMPTY_SHAPES)('enabled: $label → not greyed out', ({ value }) => {
+    renderLeaf({ ...ACT, enabled: value });
+    expect(act()).not.toBeDisabled();
+  });
+
+  it('CHANGED: enabled: \'   \' greyed the button out before this ruling', () => {
+    // The negated leg (`disabled = !isEnabled`) turned the whitespace string's
+    // falsy verdict into "disabled". Covered by the row above; stated separately
+    // because it is a behaviour change, not a preserved one.
+    renderLeaf({ ...ACT, enabled: '   ' });
+    expect(act()).not.toBeDisabled();
+  });
+
+  it('enabled: false → still greyed out (unchanged)', () => {
+    renderLeaf({ ...ACT, enabled: false });
+    expect(act()).toBeDisabled();
+  });
+
+  it('enabled: true → not greyed out (unchanged)', () => {
+    renderLeaf({ ...ACT, enabled: true });
+    expect(act()).not.toBeDisabled();
+  });
+
+  it('`disabled` still wins over `enabled` when it is declared', () => {
+    renderLeaf({ ...ACT, disabled: true, enabled: true });
+    expect(act()).toBeDisabled();
+  });
+
+  it("an EMPTY `disabled` no longer short-circuits the chain — `enabled: false` is reached", () => {
+    // The precedence case: with `disabled: ''` no longer a gate, the chain falls
+    // through to the legacy leg instead of stopping at an empty predicate.
+    renderLeaf({ ...ACT, disabled: '', enabled: false });
+    expect(act()).toBeDisabled();
+  });
+
+  it('an empty ENVELOPE `disabled` falls through the same way (objectui#3850 row)', () => {
+    const { unmount } = renderLeaf({ ...ACT, disabled: { dialect: 'cel', source: '' }, enabled: false });
+    expect(act()).toBeDisabled();
+    unmount();
+    // The other direction is the mutation detector of the pair: under the old
+    // scope the envelope was a declared gate whose verdict was `true`, so the
+    // button was greyed out no matter what `enabled` said.
+    renderLeaf({ ...ACT, disabled: { dialect: 'cel', source: '' }, enabled: true });
+    expect(act()).not.toBeDisabled();
+  });
+});