fix(app-shell,components): 动作 disabled 的「已声明」判定补上 !== '' 半边,disabled: '' 不再永久置灰 (#3842)

.changeset/action-declared-disabled-gate-3842.md

@@ -0,0 +1,52 @@
+---
+"@object-ui/app-shell": patch
+"@object-ui/components": patch
+---
+
+An action declaring `disabled: ''` is no longer greyed out forever (objectui#3842)
+
+The "is a `disabled` gate declared?" test stopped at `!= null`, missing the
+`!== ''` half of the invariant the `visible` family converged on
+(`hasDeclaredVisibilityGate`, objectui#3492 / #3758 / #3812 / #3823 / #3835). So
+`disabled: ''` counted as a declared gate, and the verdict went to the evaluation
+entry — which reads an empty predicate as "no condition → `true`"
+(`toPredicateInput('')` is `undefined`, `evaluateCondition(undefined)` is `true`).
+
+The direction is why this half is a defect and the `visible` half was not. On
+`visible`, that `true` means SHOW, so an over-broad "declared" test and a
+permissive empty predicate cancel out and `visible: ''` renders either way. On
+`disabled`, the same `true` means DISABLE — the two mistakes compound, and an
+empty predicate stopped meaning "no gate" and started meaning "permanently
+greyed out". One empty predicate, opposite treatment under two keys.
+
+Two gates now ask the shared definition instead:
+
+- `@object-ui/app-shell`'s `DeclaredActionsBar` — the hot one. Its actions are
+  SERVER-declared (`objectDef.actions[]`) and its hosts are the approvals inbox's
+  record sections, so a `disabled: ''` arriving from metadata (an authoring form
+  left empty, a template that rendered to an empty string) produced an Approve /
+  Reject button nobody could click, indistinguishable from deliberate metadata.
+  objectui#3835 was this same surface failing the other way.
+- `@object-ui/components`' `action:button` — verified to be the same shape before
+  it was changed (the issue inferred it from the identical spelling but did not
+  probe it): with `disabled: ''` the rendered button carried `disabled=""`.
+
+**Behaviour change surface, deliberately narrow.** Only `disabled: ''` changes —
+from disabled to clickable, which is what "no predicate" asked for. `disabled:
+true` still disables, `disabled: false` and an absent `disabled` still do not, and
+no expression-valued `disabled` changes verdict. One consequence worth naming: on
+`action:button`, an empty `disabled` now falls THROUGH to the legacy non-spec
+`enabled` fallback instead of short-circuiting on the empty predicate, so an
+action spelling both (`disabled: ''` + `enabled: true`) becomes clickable.
+
+The legacy `enabled` leg of `action:button` was routed through the same
+definition for consistency, and that part is behaviour-preserving by derivation
+rather than a fix: the leg is negated (`disabled = !isEnabled`), so an empty
+predicate's `true` already arrived as "not disabled" — the same verdict "no gate"
+produces. All four shapes are identical under either test; the derivation table
+and the reason no test can distinguish them are written down next to the pins.
+
+`hasDeclaredVisibilityGate` keeps its historic name at both call sites (the
+objectui#3842 dispatch ruling): the predicate is key-neutral, and one
+implementation behind two names is how a repo grows dialects. Each call site says
+so in a comment.

packages/app-shell/src/views/DeclaredActionsBar.tsx

@@ -227,7 +227,23 @@ const DeclaredActionButton: React.FC<{
       type="button"
       size="sm"
       variant={variant as any}
-      disabled={((action as any).disabled != null ? isDisabledPred : false) || loading}
+      // Is a `disabled` gate DECLARED? The same question the `visible` gate
+      // above asks, so it reads the same definition rather than re-spelling it.
+      // The name is historic — objectui#3492 arrived through `visible` — and the
+      // predicate is key-neutral: "declared" is `!= null && !== ''`, because an
+      // empty predicate is nothing to evaluate. Kept under that name
+      // deliberately (objectui#3842 ruling): one implementation behind two names
+      // is a dialect, not a clarification.
+      //
+      // `!= null` alone was a real defect here, and NOT for the reason it was on
+      // `visible`: the evaluation entry reads an empty predicate as "no
+      // condition → true", which on `visible` means SHOW (so an over-broad
+      // "declared" test cancels out and `''` renders either way), but here means
+      // DISABLE. A `disabled: ''` on a server-declared approval action rendered
+      // a permanently greyed-out Approve / Reject — the mirror image of
+      // objectui#3835 on the same surface, and equally impossible to tell from
+      // deliberate metadata by looking at it.
+      disabled={(hasDeclaredVisibilityGate((action as any).disabled) ? isDisabledPred : false) || loading}
       onClick={handleClick}
       data-testid={`declared-action-${action.name}`}
     >

packages/app-shell/src/views/__tests__/DeclaredActionsBar.test.tsx

@@ -529,3 +529,86 @@ describe('DeclaredActionsBar — declared `visible` on a server-declared action
     expect(executeSpy.mock.calls[0][0]).toMatchObject({ name: 'approval_reassign' });
   });
 });
+
+/**
+ * objectui#3842 — the declared `disabled` gate on the same surface, and the
+ * half of the objectui#3492 family where the `''` reasoning INVERTS.
+ *
+ * The gate asked `(action as any).disabled != null`, so `disabled: ''` counted
+ * as "a gate is declared" and the verdict went to the evaluation entry — which
+ * reads an empty predicate as "no condition → true"
+ * (`toPredicateInput('')` → `undefined` → `evaluateCondition(undefined)` →
+ * `true`). Direction is everything: on `visible` that `true` means SHOW, so the
+ * over-broad "declared" test and the permissive empty predicate cancel out
+ * (which is why the `visible: ''` case above is documentation, not a detector,
+ * and says so). On `disabled` the same `true` means DISABLE, so the two
+ * compound: an empty predicate stopped meaning "no gate" and started meaning
+ * "greyed out forever".
+ *
+ * What that costs on THIS surface: the actions are server-declared
+ * (`objectDef.actions[]`) and the bar's own hosts are the approvals inbox's
+ * record sections, so a `disabled: ''` — an authoring form left empty, a
+ * template that rendered to an empty string — greys out Approve / Reject with
+ * no way for the approver to tell metadata intent from a bug. objectui#3835 was
+ * the same surface failing the other way (hidden action still clickable).
+ *
+ * `''` IS a mutation detector here, unlike on `visible`: restore `!= null` and
+ * this block's first case goes red on its own. All four shapes are asserted
+ * because "never disable anything" satisfies three of them, and `disabled: true`
+ * is what refuses that rewrite.
+ *
+ * No legacy `enabled` leg on this surface by design (see the component: server-
+ * declared actions are spec-shaped and never carried the non-spec key) — that
+ * leg's four shapes are pinned in `packages/components`, next to the renderer
+ * that has it.
+ */
+describe('DeclaredActionsBar — declared `disabled` on a server-declared action (objectui#3842)', () => {
+  const APPROVE = {
+    name: 'approval_approve',
+    type: 'api',
+    label: 'Approve',
+    target: '/api/v1/approvals/requests/{id}/approve',
+    locations: ['record_section'],
+  };
+  const approve = () => screen.getByTestId('declared-action-approval_approve');
+
+  it("an empty-string `disabled` is not a declared gate — Approve stays clickable", () => {
+    renderWithGate({ ...APPROVE, disabled: '' });
+    expect(approve()).not.toBeDisabled();
+  });
+
+  it('disabled:true → Approve is disabled', () => {
+    renderWithGate({ ...APPROVE, disabled: true });
+    expect(approve()).toBeDisabled();
+  });
+
+  it('disabled:false → Approve is not disabled', () => {
+    renderWithGate({ ...APPROVE, disabled: false });
+    expect(approve()).not.toBeDisabled();
+  });
+
+  it('no `disabled` at all → Approve is not disabled (ungated stays ungated)', () => {
+    renderWithGate({ ...APPROVE });
+    expect(approve()).not.toBeDisabled();
+  });
+
+  it('an expression-valued `disabled` keeps its verdict — true disables, false does not', () => {
+    const gated = { ...APPROVE, disabled: 'status == "approved"' };
+    const { unmount } = renderWithGate(gated, { ...REQUEST, status: 'approved' });
+    expect(approve()).toBeDisabled();
+    unmount();
+    renderWithGate(gated, { ...REQUEST, status: 'pending' });
+    expect(approve()).not.toBeDisabled();
+  });
+
+  it('an empty-string `disabled` leaves an Approve that actually DISPATCHES', async () => {
+    // Not-disabled is only the visible half of the claim. The `disabled`
+    // attribute is what this component hands its own click handler — the one
+    // that POSTs the approve call — so the case worth pinning is that the
+    // button reaches `execute`, not merely that an attribute is absent.
+    renderWithGate({ ...APPROVE, disabled: '' });
+    fireEvent.click(approve());
+    await waitFor(() => expect(executeSpy).toHaveBeenCalledTimes(1));
+    expect(executeSpy.mock.calls[0][0]).toMatchObject({ name: 'approval_approve' });
+  });
+});

packages/components/src/renderers/action/__tests__/action-disabled-declared-gate.test.tsx

@@ -0,0 +1,158 @@
+/**
+ * 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#3842 — the declared-`disabled` gate. The `disabled` half of the
+ * objectui#3492 family, and the half where the family's usual "`''` is
+ * harmless" reasoning INVERTS.
+ *
+ * The gate asked `(schema as any).disabled != null`, so `disabled: ''` counted
+ * as "a gate is declared" and the verdict was handed to the evaluation entry —
+ * which reads an empty predicate as "nothing to evaluate → true"
+ * (`toPredicateInput('')` is `undefined`, `evaluateCondition(undefined)` is
+ * `true`). On `visible` that `true` means SHOW, so an over-broad "declared"
+ * test and a permissive empty predicate cancel out and `visible: ''` renders
+ * either way (which is why PR #3843 could only document `''` on that side, not
+ * detect a mutation with it). On `disabled` the same `true` means DISABLE, so
+ * the two mistakes compound instead of cancelling: an empty predicate stopped
+ * meaning "no gate" and started meaning "greyed out forever".
+ *
+ * Both legs now ask `hasDeclaredVisibilityGate` (`!= null && !== ''`) — the one
+ * definition, imported rather than re-spelled, historic name kept per the
+ * objectui#3842 dispatch ruling (see the comment at the gate).
+ *
+ * ## What each case detects
+ *
+ *   • `disabled: ''` → NOT disabled. THE defect, and on this side a genuine
+ *     mutation detector: restore `!= null` and this case alone goes red.
+ *   • `disabled: true` → disabled, and `disabled: false` / undeclared → not
+ *     disabled. Anti-mutation guards: "never disable anything" satisfies three
+ *     of the four shapes on its own, and `true` is what refuses it.
+ *   • expression-valued `disabled` → the verdict still decides, both ways. The
+ *     gate narrowed; evaluation did not change.
+ *
+ * ## The legacy `enabled` leg, and why its four cases are documentation
+ *
+ * The leg is NEGATED (`disabled = !isEnabled`), so the empty predicate's `true`
+ * arrives as `!true` = "not disabled" — which is exactly what "no gate
+ * declared" produces. Every shape therefore reaches the same verdict under
+ * either test, and the tightening is behaviour-preserving by derivation:
+ *
+ *   | `enabled`   | `!= null` (old)          | `hasDeclaredVisibilityGate` (new) |
+ *   |-------------|--------------------------|-----------------------------------|
+ *   | `''`        | gate → `!true`  = enabled | no gate → `false` = enabled       |
+ *   | `true`      | gate → `!true`  = enabled | gate → `!true`  = enabled         |
+ *   | `false`     | gate → `!false` = DISABLED| gate → `!false` = DISABLED        |
+ *   | undeclared  | no gate → `false`         | no gate → `false`                 |
+ *
+ * So no `enabled` case here can go red by reverting the `enabled` leg — stated
+ * plainly rather than dressed up as coverage. They are kept because they pin
+ * the semantics the derivation asserts (`enabled: false` must still disable),
+ * which is what a future rewrite of this chain would break silently. The one
+ * case that DOES move is the precedence case below: with `disabled: ''` no
+ * longer a gate, the chain falls through to the legacy leg instead of
+ * short-circuiting on an empty predicate.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { render, screen } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import React from 'react';
+import { ComponentRegistry } 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 §测试纪律 — the cost lands in
+// the import phase, not under a hook timeout.
+import '../action-button';
+
+/** Mount the leaf the way `action:bar` mounts it: whole action spread onto `schema`. */
+function renderLeaf(action: any, scope: Record<string, any> = {}) {
+  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: action.type }} />
+    </PredicateScopeProvider>,
+  );
+}
+
+const ACT = { name: 'act', label: 'Act', type: 'script' };
+const act = () => screen.getByRole('button', { name: 'Act' });
+
+describe('action:button — declared `disabled` gate (objectui#3842)', () => {
+  it("an empty-string `disabled` is not a declared gate — the button stays clickable", () => {
+    renderLeaf({ ...ACT, disabled: '' });
+    expect(act()).not.toBeDisabled();
+  });
+
+  it('disabled:true → the button is disabled', () => {
+    renderLeaf({ ...ACT, disabled: true });
+    expect(act()).toBeDisabled();
+  });
+
+  it('disabled:false → the button is not disabled', () => {
+    renderLeaf({ ...ACT, disabled: false });
+    expect(act()).not.toBeDisabled();
+  });
+
+  it('no `disabled` at all → the button is not disabled (ungated stays ungated)', () => {
+    renderLeaf({ ...ACT });
+    expect(act()).not.toBeDisabled();
+  });
+
+  it('an expression-valued `disabled` keeps its verdict — true disables, false does not', () => {
+    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 — legacy `enabled` leg (objectui#3842)', () => {
+  it("an empty-string `enabled` is not a declared gate — the button stays clickable", () => {
+    renderLeaf({ ...ACT, enabled: '' });
+    expect(act()).not.toBeDisabled();
+  });
+
+  it('enabled:false → the button is disabled (the legacy leg still decides)', () => {
+    renderLeaf({ ...ACT, enabled: false });
+    expect(act()).toBeDisabled();
+  });
+
+  it('enabled:true → the button is not disabled', () => {
+    renderLeaf({ ...ACT, enabled: true });
+    expect(act()).not.toBeDisabled();
+  });
+
+  it('no `enabled` at all → the button is not disabled', () => {
+    renderLeaf({ ...ACT });
+    expect(act()).not.toBeDisabled();
+  });
+
+  it('an empty `disabled` falls THROUGH to the legacy `enabled` leg', () => {
+    // The precedence case, and the second one the narrowing moves: `disabled:
+    // ''` used to short-circuit the chain on an empty predicate (→ disabled),
+    // so the author's `enabled: true` was never consulted. With `''` no longer
+    // a gate, the legacy leg decides — "no `disabled` declared" means exactly
+    // that, whatever else the action declares.
+    renderLeaf({ ...ACT, disabled: '', enabled: true });
+    expect(act()).not.toBeDisabled();
+  });
+
+  it('an empty `disabled` does not mask a legacy `enabled: false`', () => {
+    // Same fall-through, opposite verdict: the legacy leg is reached and says
+    // disabled. Green before and after (the old chain also disabled, for the
+    // wrong reason) — it is here so the fall-through cannot be "read the
+    // `enabled` leg only when it agrees with not-disabled".
+    renderLeaf({ ...ACT, disabled: '', enabled: false });
+    expect(act()).toBeDisabled();
+  });
+});

packages/components/src/renderers/action/action-button.tsx

@@ -160,10 +160,30 @@ const ActionButtonRenderer = forwardRef<HTMLButtonElement, ActionButtonProps>(
         variant={variant as any}
         size={size as any}
         className={cn(schema.className, className)}
+        // Is a `disabled` / `enabled` gate DECLARED? Same question as the
+        // `visible` gate above, so it reads the same definition — the name is
+        // historic (objectui#3492 arrived through `visible`); the predicate is
+        // key-neutral: "declared" means `!= null && !== ''`, an empty predicate
+        // is nothing to evaluate and therefore no gate. Deliberately NOT
+        // renamed or aliased for this call site (objectui#3842 ruling): one
+        // implementation under two names is how a repo grows dialects.
+        //
+        // `!= null` alone was a live defect here in a way it is not on
+        // `visible` (objectui#3842). The evaluation entry reads an empty
+        // predicate as "no condition → true"; on `visible` that `true` means
+        // SHOW, so an over-broad "declared" test cancels out, while here it
+        // means DISABLE — `disabled: ''` was a permanently greyed-out button.
+        //
+        // The legacy `enabled` leg is negated (`!isEnabled`), so its `''` case
+        // already landed on "not disabled" by double negation; routing it
+        // through the same definition is behaviour-preserving for all four
+        // shapes (derivation table in
+        // `__tests__/action-disabled-declared-gate.test.tsx`) and keeps one
+        // spelling of "declared" on both legs.
         disabled={(
-          (schema as any).disabled != null
+          hasDeclaredVisibilityGate((schema as any).disabled)
             ? isDisabled
-            : schema.enabled != null
+            : hasDeclaredVisibilityGate(schema.enabled)
               ? !isEnabled
               : false
         ) || loading}