fix(app-shell,components): 服务端声明动作的 visible 门按「已声明」判定,visible: false 不再渲染 (#3835)

.changeset/components-export-declared-visibility-gate-3835.md

@@ -0,0 +1,21 @@
+---
+"@object-ui/components": patch
+---
+
+Export `hasDeclaredVisibilityGate` from the package barrel (objectui#3835)
+
+`hasDeclaredVisibilityGate(visible)` — "did this action DECLARE a visibility gate
+at all?", i.e. `!= null && !== ''`, with the verdict left to the evaluation entry
+— is the single definition objectui#3492 established and PR #3816 / #3825 / #3836
+applied to every member-action gate in this package and in `@object-ui/plugin-grid`.
+It lived module-private in `src/renderers/action/visibility-gate.ts`.
+
+The family turned out to have a member outside these packages:
+`@object-ui/app-shell`'s `DeclaredActionsBar` gates server-declared actions with
+the same question and had the same truthiness bug (objectui#3835). Exporting the
+one definition is what keeps that fix from becoming a fifth hand-spelled copy of
+it — the drift shape objectui#3142 already had to unpick for `locations` in these
+same files.
+
+Additive only: one `export` line, no behaviour change in this package. The
+function is pure and dependency-free.

.changeset/declared-actions-bar-visible-gate-3835.md

@@ -0,0 +1,58 @@
+---
+"@object-ui/app-shell": patch
+---
+
+Server-declared actions declaring `visible: false` are now hidden instead of rendered as live buttons (objectui#3835)
+
+`DeclaredActionsBar` — the bar that renders an object's SERVER-declared actions
+for one record at a `location`, with no per-action host code — asked truthiness
+on the gate: `if (action.visible && !isVisible) return null`. `false && …` is
+falsy, so `visible: false`, the most explicit way an author can say "never show
+this", fell into the "no gate declared" branch, the verdict was never consulted,
+and the action rendered for everyone.
+
+What that means on the page: the bar's host is the approvals inbox's
+record-section toolbar (`apps/console/src/pages/system/ApprovalsInboxPage.tsx`),
+so an approval action the metadata had switched off with `visible: false`
+rendered as a live Approve / Reject / Reassign button — and this component's own
+click handler is what POSTs the decision. One click was a real approve/reject
+call on a request the declaration said not to offer a decision on.
+
+This is the fifth and last member of the objectui#3492 family (after
+objectui#3758 / PR #3816 for the row-action surfaces and objectui#3812 / #3823
+for the action face), and the one whose two family-wide mitigations both fail:
+
+- The action defs are **server-declared** (`objectDef.actions[]`,
+  `sys_approval_request`), not hand-written view JSON. "`ActionSchema.visible` is
+  `ExpressionInputSchema` with no boolean member, so `objectstack build` cannot
+  emit this shape" does not apply on this path — the def arrives from server
+  metadata and in-process construction, where a boolean is the natural spelling.
+- The bar is mounted as **plain JSX** by its hosts, so `packages/react`'s
+  `SchemaRenderer` — which evaluates a node's `visible` and hides it before the
+  component mounts, and which is why objectui#3812 judged the component-level
+  gates a dormant defensive layer — is not on this path at all. This gate was the
+  only one there.
+
+The gate now reads the family's one named definition,
+`hasDeclaredVisibilityGate` (`!= null && !== ''`), imported from
+`@object-ui/components` rather than re-spelled: five gates in three packages
+asking one question must not drift into five answers. The evaluation entry is
+untouched — `toPredicateInput` passes a boolean through and `useCondition`
+short-circuits it instead of calling the expression engine — so a declared
+`false` resolves to `false`, and every expression-valued `visible` keeps exactly
+the verdict it had.
+
+Behaviour change surface, deliberately narrow: only a declared action whose
+`visible` is the literal boolean `false` (or another falsy non-empty value)
+changes, from rendered to hidden, which is what the declaration asked for.
+`visible: true` still renders, `''` and an absent `visible` are still no gate at
+all, and the bar still renders no chrome when its located set is empty.
+
+The suite that covered this component could not have caught it: it stubbed the
+whole predicate entry constant-true (`useCondition: () => true`), with a comment
+saying the test actions omit `visible` "so this is unused" — which made the gate
+unreachable from the only tests that mount this component (the objectstack#4984
+family, where a fixture keeps a broken rule green). That stub is gone; the suite
+now runs the real `useCondition` / `toPredicateInput` and doubles only the action
+dispatch, so all four shapes (`false` hides / `true` renders / undeclared renders
+/ `''` is not a gate) are judged by the shipped evaluation semantics.

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

@@ -31,7 +31,7 @@
  */
 
 import React, { useCallback, useMemo, useState } from 'react';
-import { Button, Separator, cn } from '@object-ui/components';
+import { Button, Separator, cn, hasDeclaredVisibilityGate } from '@object-ui/components';
 import {
   ActionProvider,
   useAction,
@@ -187,7 +187,25 @@ const DeclaredActionButton: React.FC<{
     }
   }, [action, execute, loading, objectName, record, actionLabel, actionConfirm, actionSuccess, t]);
 
-  if ((action as any).visible && !isVisible) return null;
+  // Does the action DECLARE a `visible` gate? `hasDeclaredVisibilityGate`
+  // (`!= null && !== ''`) is the one definition on the question, imported rather
+  // than re-spelled. This gate used to ask truthiness, which classified
+  // `visible: false` — the most explicit "never show this" an author can write —
+  // as "no gate declared", skipped the verdict, and rendered the action for
+  // everyone (objectui#3835, the fifth member of the objectui#3492 family).
+  //
+  // The stakes here are the highest of the family: the actions are
+  // SERVER-declared (`objectDef.actions[]`), so "the spec's `visible` has no
+  // boolean member, `objectstack build` cannot emit one" does not apply, and this
+  // bar is mounted as plain JSX by its hosts — `packages/react`'s
+  // `SchemaRenderer`, which hides a `visible`-carrying node before its component
+  // mounts, is not on this path. This is the only gate on it, in front of the
+  // approvals inbox's Approve / Reject buttons.
+  //
+  // The verdict stays with the evaluation entry above: `toPredicateInput` passes
+  // a boolean through untouched and `useCondition` short-circuits it instead of
+  // calling the expression engine, so a declared `false` is `false`.
+  if (hasDeclaredVisibilityGate((action as any).visible) && !isVisible) return null;
 
   const iconName = typeof (action as any).icon === 'string' ? (action as any).icon as string : undefined;
   // Map the spec's action `variant` enum (primary|secondary|danger|ghost|link)

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

@@ -18,14 +18,28 @@ import React from 'react';
 // Capture the execute dispatch from the shared runner.
 const executeSpy = vi.fn().mockResolvedValue({ success: true });
 
-vi.mock('@object-ui/react', () => ({
-  ActionProvider: ({ children }: { children: React.ReactNode }) => <>{children}</>,
-  useAction: () => ({ execute: executeSpy }),
-  // `visible` predicate: our test actions omit `visible`, so this is unused,
-  // but keep it truthy so a `visible`-carrying action would still render.
-  useCondition: () => true,
-  toPredicateInput: (v: unknown) => v,
-}));
+// Only the action DISPATCH is doubled here. The predicate entry
+// (`useCondition` / `toPredicateInput`) is the REAL one (objectui#3835).
+//
+// It used to be stubbed constant-true, with a comment saying our test actions
+// omit `visible` "so this is unused" — which made the component's `visible`
+// gate unreachable from this suite for as long as it existed, so the truthiness
+// bug objectui#3835 reports lived here untouched (the objectstack#4984 family: a
+// fixture keeping a broken rule green). A re-spelled stub would not fix that:
+// `visible: false` only reaches "hidden" if `toPredicateInput` passes the
+// boolean through and `evaluateCondition` short-circuits it, so a stub is a
+// second copy of exactly the semantics under test. `@object-ui/react`'s barrel
+// is cheap for the light `dom` project (`packages/components`' own gate suite
+// imports it unmocked), so the gate below is judged by the shipped evaluation
+// entry instead.
+vi.mock('@object-ui/react', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('@object-ui/react')>();
+  return {
+    ...actual,
+    ActionProvider: ({ children }: { children: React.ReactNode }) => <>{children}</>,
+    useAction: () => ({ execute: executeSpy }),
+  };
+});
 
 // The runtime is exercised in its own suite; here it's an inert shell so the
 // bar mounts without the full auth/i18n/router provider stack.
@@ -54,13 +68,27 @@ vi.mock('@object-ui/i18n', () => ({
   useObjectTranslation: () => ({ t: (key: string) => `t:${key}` }),
 }));
 
-vi.mock('@object-ui/components', () => ({
-  Button: ({ children, onClick, ...props }: any) => (
-    <button onClick={onClick} {...props}>{children}</button>
-  ),
-  Separator: () => <hr />,
-  cn: (...args: any[]) => args.filter(Boolean).join(' '),
-}));
+// The components barrel stays doubled (its full graph is what the light `dom`
+// project deliberately does not load), but `hasDeclaredVisibilityGate` is pulled
+// from its real source module — the ONE definition objectui#3492 established and
+// PR #3816 / #3825 / #3836 spread across the other four member-action gates. A
+// re-spelled `v != null && v !== ''` here would be a fifth copy of it living in
+// a test double, and would keep this suite green no matter what the shipped
+// predicate does (objectui#3142 is what copies of one answer cost). The module
+// is a dependency-free pure function, so importing it directly costs nothing.
+vi.mock('@object-ui/components', async () => {
+  const { hasDeclaredVisibilityGate } = await import(
+    '../../../../components/src/renderers/action/visibility-gate'
+  );
+  return {
+    Button: ({ children, onClick, ...props }: any) => (
+      <button onClick={onClick} {...props}>{children}</button>
+    ),
+    Separator: () => <hr />,
+    cn: (...args: any[]) => args.filter(Boolean).join(' '),
+    hasDeclaredVisibilityGate,
+  };
+});
 
 import { DeclaredActionsBar } from '../DeclaredActionsBar';
 
@@ -372,3 +400,132 @@ describe('DeclaredActionsBar chrome localization (objectui#2762)', () => {
     expect(byName['outputs.notes'].helpText).toBe('t:actions.decisionOutput.helpMultiValue');
   });
 });
+
+/**
+ * objectui#3835 — the declared-action `visible` gate. Fifth and hottest member
+ * of the objectui#3492 family: `if (action.visible && !isVisible) return null`
+ * asked TRUTHINESS, so `visible: false` — the most explicit "never show this" an
+ * author can write — was classified as "no gate declared", the verdict was never
+ * consulted, and the action rendered for everyone.
+ *
+ * Why this surface is the hot one, and why the family's mitigation does not
+ * cover it:
+ *
+ *   • The actions are SERVER-DECLARED (`objectDef.actions[]` /
+ *     `sys_approval_request`), not hand-written view JSON, so "spec's
+ *     `ExpressionInputSchema` has no boolean member, `objectstack build` cannot
+ *     emit one" does not apply — the def arrives from server metadata and
+ *     in-process construction, where a boolean is the natural spelling.
+ *   • `DeclaredActionsBar` is mounted as plain JSX by its hosts
+ *     (`apps/console/src/pages/system/ApprovalsInboxPage.tsx:2014` and `:2055`),
+ *     so `packages/react`'s `SchemaRenderer` — which hides a node whose
+ *     `visible !== undefined` evaluates false, and which made the component-level
+ *     gates of objectui#3812 dormant — is not on this path at all. This gate is
+ *     the only one there.
+ *   • What renders is the approvals inbox's record-section bar: Approve / Reject
+ *     / Reassign. A `visible: false` approval action rendered as a live button is
+ *     one click away from a real approve/reject call.
+ *
+ * The gate now asks `hasDeclaredVisibilityGate` (`!= null && !== ''`), imported
+ * from `@object-ui/components` rather than re-spelled — the verdict stays with
+ * the evaluation entry, which short-circuits a boolean instead of calling the
+ * expression engine.
+ *
+ * All FOUR shapes are asserted, and each one is load-bearing in a different
+ * direction:
+ *   • `false` hides — the defect itself (red before the fix);
+ *   • `true` renders and undeclared renders — the anti-mutation guards: "hide
+ *     the action unconditionally" satisfies every `visible: false` assertion on
+ *     its own and would otherwise leave the suite green;
+ *   • `''` renders — green BEFORE and AFTER the fix, and (measured, not assumed)
+ *     green even under a gate mutated to `visible !== undefined`. On THIS
+ *     surface `''` is covered twice: over-tightening the gate hands `''` to the
+ *     evaluation entry, and `toPredicateInput('')` is `undefined`, which
+ *     `evaluateCondition` reads as "no condition → visible". So this case
+ *     documents the intended semantics; it is not a mutation detector here, and
+ *     a reader should not mistake its passing for proof that the gate's `!== ''`
+ *     limb is exercised (the limb itself is pinned in `packages/components`,
+ *     next to the definition).
+ * Every case carries the ungated `COMPANION`, so a passing "not rendered" can
+ * never mean "the whole bar returned null" (the bar renders nothing at all when
+ * its located set is empty — a distinct code path, two lines away).
+ */
+const COMPANION = {
+  name: 'approval_reassign',
+  type: 'api',
+  label: 'Reassign',
+  target: '/api/v1/approvals/requests/{id}/reassign',
+  locations: ['record_section'],
+};
+
+function renderWithGate(action: Record<string, unknown>, record: Record<string, unknown> = REQUEST) {
+  return render(
+    <DeclaredActionsBar
+      objectName="sys_approval_request"
+      record={record}
+      location="record_section"
+      actions={[action, COMPANION] as any}
+    />,
+  );
+}
+
+describe('DeclaredActionsBar — declared `visible` on a server-declared action (objectui#3835)', () => {
+  const APPROVE = {
+    name: 'approval_approve',
+    type: 'api',
+    label: 'Approve',
+    target: '/api/v1/approvals/requests/{id}/approve',
+    locations: ['record_section'],
+  };
+
+  it('visible:false → the declared action does not render', () => {
+    renderWithGate({ ...APPROVE, visible: false });
+    expect(screen.queryByTestId('declared-action-approval_approve')).toBeNull();
+    // The bar itself rendered — the assertion above is about the gate, not about
+    // an empty located set.
+    expect(screen.getByTestId('declared-action-approval_reassign')).toBeInTheDocument();
+  });
+
+  it('visible:true → the declared action renders', () => {
+    renderWithGate({ ...APPROVE, visible: true });
+    expect(screen.getByTestId('declared-action-approval_approve')).toBeInTheDocument();
+    expect(screen.getByTestId('declared-action-approval_reassign')).toBeInTheDocument();
+  });
+
+  it('no `visible` at all → the declared action renders (ungated stays ungated)', () => {
+    renderWithGate({ ...APPROVE });
+    expect(screen.getByTestId('declared-action-approval_approve')).toBeInTheDocument();
+  });
+
+  it('an empty-string `visible` is not a declared gate — the action still renders', () => {
+    renderWithGate({ ...APPROVE, visible: '' });
+    expect(screen.getByTestId('declared-action-approval_approve')).toBeInTheDocument();
+  });
+
+  it('an expression-valued `visible` keeps its verdict — false hides, true shows', () => {
+    const gated = { ...APPROVE, visible: 'status == "pending"' };
+    const { unmount } = renderWithGate(gated, { ...REQUEST, status: 'approved' });
+    expect(screen.queryByTestId('declared-action-approval_approve')).toBeNull();
+    expect(screen.getByTestId('declared-action-approval_reassign')).toBeInTheDocument();
+    unmount();
+    renderWithGate(gated, { ...REQUEST, status: 'pending' });
+    expect(screen.getByTestId('declared-action-approval_approve')).toBeInTheDocument();
+  });
+
+  it('a hidden action leaves NO clickable surface in the toolbar', async () => {
+    // Hiding is not cosmetic on this surface: this component's own click handler
+    // is what POSTs the approve/reject call, so what matters is that the gated
+    // action contributes no button at all — not merely that a query by testid
+    // misses it. Asserting the toolbar's button SET (rather than clicking the
+    // companion and counting dispatches) is what makes this case move: an
+    // unclicked extra button dispatches nothing either way, so a
+    // dispatch-counting version of this test was green before the fix too.
+    renderWithGate({ ...APPROVE, visible: false });
+    const buttons = screen.getByRole('toolbar').querySelectorAll('button');
+    expect(buttons).toHaveLength(1);
+    expect(buttons[0]).toHaveAttribute('data-testid', 'declared-action-approval_reassign');
+    fireEvent.click(buttons[0]);
+    await waitFor(() => expect(executeSpy).toHaveBeenCalledTimes(1));
+    expect(executeSpy.mock.calls[0][0]).toMatchObject({ name: 'approval_reassign' });
+  });
+});

packages/components/src/index.ts

@@ -41,6 +41,21 @@ export { renderChildren } from './lib/utils';
 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`.
+//
+// Exported because the family has a member OUTSIDE these packages: app-shell's
+// `DeclaredActionsBar` mounts an object's server-declared actions as plain JSX
+// (no `SchemaRenderer` in front, so its own gate is the only one on that path),
+// and asked truthiness — `visible: false` rendered a live Approve/Reject button
+// (objectui#3835). A re-export, not a copy: five gates in three packages asking
+// one question must not drift into five answers, which is exactly the shape
+// objectui#3142 had to unpick for `locations` in these same files. app-shell
+// already depends on this package, so the direction costs nothing new.
+export { hasDeclaredVisibilityGate } from './renderers/action/visibility-gate';
+
 // Export placeholder registration
 export { registerPlaceholders } from './renderers/placeholders';