diff --git a/.changeset/7234-requiredpermissions-author-channel.md b/.changeset/7234-requiredpermissions-author-channel.md new file mode 100644 index 0000000000..5a568cb4c8 --- /dev/null +++ b/.changeset/7234-requiredpermissions-author-channel.md @@ -0,0 +1,36 @@ +--- +'@object-ui/app-shell': minor +--- + +Say why a capability-gated action is missing, in the action designer (objectui#7234, +maintainer ruling 2026-09-08, option B). + +`action.requiredPermissions` (ADR-0066 D4) is enforced with a 403 on the platform +action route and mirrored as a UI hide. The mirror is silent by design: a viewer who +does not hold every listed capability gets no button, no greyed-out control and no +message, at every declared location at once. An admin who had configured a set of +object-bound buttons, held no permission set yet, and opened the list saw nothing at +all — and read it as a broken feature, because nothing in the product said otherwise. + +**End-user behaviour is unchanged, deliberately.** The action stays hidden, no new +end-user surface is added, and drawing the action greyed out with the missing +capability named was considered and rejected — it advertises to end users capabilities +they do not have. `ObjectView.objectBoundActions-7234.test.tsx` now pins that the +gated end-user surface explains nothing, so landing this reason on a running app's +list surface turns a test red. + +What changes is the author-facing side, in the panel pair the Studio Data tab's +**Actions** config already uses: + +- The inspector's **Placement** section, beside the existing "no placement selected" + notice, names the gating capabilities and says the action is *hidden* rather than + disabled. When the signed-in session is itself missing one of them it says so in the + first person, reading the held set from `MePermissionsProvider` through + `usePermissions` — the same signal `useCanAuthorMetadata` consumes, not a second + client-side permission derivation. An unreported held set is treated as unknown and + that clause stays silent, mirroring the gate's own fail-open doctrine. +- The **action preview** carries the capability line in its metadata strip and above + its *Where it appears* frames, which previously drew the button in every declared + location without qualification. + +`content/docs/guide/console.md` states the hide and where the reason is shown. diff --git a/content/docs/guide/console.md b/content/docs/guide/console.md index 70151649ff..6199e3fc92 100644 --- a/content/docs/guide/console.md +++ b/content/docs/guide/console.md @@ -52,6 +52,35 @@ Validations and Actions persist with the object's own **Save draft**; Hooks (a distinct metadata type) save per-hook. Nothing goes live until the package is published from the top-bar **Publish** flow. +#### Why a configured action can be missing from the app + +An action that is saved, published and correctly placed can still appear on no +surface at all. There are two reasons, and the Actions panel states both — the +running app deliberately states neither, because an end user should not be told +about capabilities they do not have. + +* **No placement.** `locations` is empty, so the action surfaces nowhere. + A selection-only action is legitimate here: it is placed by a list view's + `bulkActions` / `bulkActionDefs` instead. +* **The capability gate.** `action.requiredPermissions` (ADR-0066 D4) is + enforced with a 403 on the platform action route and **mirrored as a UI + hide**: a viewer who does not hold every listed capability gets no button, + no greyed-out control and no message, at every declared location at once. + This is intended — the client hide mirrors an enforcement the server applies + regardless, and an unentitled user is never shown a control they cannot use. + +The **Placement** section of the action inspector names the gating capabilities +and says the action is hidden rather than disabled; when the signed-in session +is itself missing one of them it says so too, so "I configured the buttons and +I see none of them" has an answer on the screen where the buttons were +configured. The read-only **action preview** beside it carries the same +capability line, so its *Where it appears* frames are not read as a promise +that everyone will see the button there. + +An unheld capability is not the same problem as a *misspelled* one: a +capability string registered nowhere is caught separately, by the advisory +capability-reference lint that runs over pending drafts during **Publish**. + ## Configuration **The console has no configuration file.** It declares no apps, objects or views of its own — diff --git a/packages/app-shell/src/views/ObjectView.objectBoundActions-7234.test.tsx b/packages/app-shell/src/views/ObjectView.objectBoundActions-7234.test.tsx index 896c778a7c..3bc3636c52 100644 --- a/packages/app-shell/src/views/ObjectView.objectBoundActions-7234.test.tsx +++ b/packages/app-shell/src/views/ObjectView.objectBoundActions-7234.test.tsx @@ -45,10 +45,24 @@ * only the held set differs. * * ⚠️ This file therefore pins CURRENT, DELIBERATE behaviour. It is not a fix - * for #7234 and does not claim one. Whether a declared-but-ungranted action - * should stay silently invisible — or be surfaced some other way, since today - * nothing anywhere tells an author why their button is missing — is the open - * question left on the card. + * for #7234 and does not claim one. + * + * ## Ruled 2026-09-08, and case J is this file's half of the ruling + * + * Whether a declared-but-ungranted action should stay silently invisible was + * the open question this file was written under. The maintainer ruled option + * B: the hide STAYS for end users, and the reason becomes visible in an + * author/admin channel that already exists — the action designer, pinned by + * `capabilityGateChannel-7234.test.tsx`. Option C (drawing the action greyed + * out with the missing capability named) was considered and REJECTED, because + * it advertises to end users capabilities they do not have. + * + * ⇒ "No end-user change" is an acceptance criterion of that ruling, not a + * side effect of it, so it gets a pin rather than a promise. Case J asserts the + * end-user list surface still explains NOTHING in the gated state: an + * implementer who lands the author-facing reason on a running app's list + * surface turns it red. F establishes the button is gone; J establishes that + * nothing took its place. * * ## Reverse verification — direction predicted before running * @@ -341,4 +355,30 @@ describe('objectui#7234 — object-declared actions reach the list toolbar', () renderList(objectsWithActions([APPLY_TO_PEOPLE_GATED])); await waitFor(() => expect(toolbarButton()).toBeTruthy()); }); + + // ── Ruling part (a): the end user is told nothing, and stays told nothing ── + + it('J: the hidden state explains NOTHING on the end-user surface', async () => { + // Case F's exact setup — the gate closed on a capability this viewer lacks. + heldCapabilities = ['duly.task.update_status']; + const { container } = renderList(objectsWithActions([APPLY_TO_PEOPLE_GATED])); + await settle(); + + expect(toolbarButton()).toBeNull(); + + // Paired positive: the list surface itself DID render. Without it an empty + // tree would satisfy every negative below for the wrong reason — the same + // asymmetry this file's ablation note states for cases B / C / F / H. + expect(screen.getByText('Catalog item')).toBeTruthy(); + + // Nothing names the capability, and no notice stands in for the button. + // Asserted over the whole rendered tree rather than one node, because the + // route this must refuse is "surface it somewhere in the running app", + // which does not commit to a location in advance. + const rendered = container.textContent ?? ''; + expect(rendered).not.toContain('duly.catalog.apply'); + expect(rendered).not.toMatch(/capabilit/i); + expect(rendered).not.toMatch(/requiredPermissions/i); + expect(rendered).not.toMatch(/permission/i); + }); }); diff --git a/packages/app-shell/src/views/metadata-admin/capabilityGateChannel-7234.test.tsx b/packages/app-shell/src/views/metadata-admin/capabilityGateChannel-7234.test.tsx new file mode 100644 index 0000000000..dbb5f1b86f --- /dev/null +++ b/packages/app-shell/src/views/metadata-admin/capabilityGateChannel-7234.test.tsx @@ -0,0 +1,208 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#7234 — the author-facing channel for the `requiredPermissions` hide. + * + * ## What this pins, and why it is a channel rather than a fix + * + * The card was opened as "object-bound actions never render". That premise was + * falsified and the falsification is pinned by + * `ObjectView.objectBoundActions-7234.test.tsx`: the relay carries + * `objectDef.actions`, the button IS drawn, and ADR-0066 D4's + * `requiredPermissions` capability gate is what hides it — at every declared + * location at once, with no error and no 4xx. + * + * The maintainer ruled option B on 2026-09-08: the hide STAYS for end users, + * and the reason becomes visible where the person who configures the app looks. + * So there is nothing to fix in the gate, and these cases are not about the + * gate's verdict at all. They pin that the action designer — the panel pair the + * console docs name as where an object's `actions[]` are configured — states + * the reason. + * + * ## The two halves are deliberately different, and that is the point + * + * • The INSPECTOR reads the live held set (`usePermissions`, the same signal + * `useCanAuthorMetadata` consumes) and can therefore answer the reported + * complaint in the first person: "I configured the buttons and I see none of + * them." Cases C/D are one differential — identical draft, identical code + * path, only the held set differs. + * • The PREVIEW is declaration-side only. It renders a draft, not a session, + * and its "Where it appears" frames draw the button in every declared + * location; before this change that was an unqualified promise, which is the + * shape `PlacementPreview`'s own `global_nav` note rules against. + * + * ## Paired assertions + * + * Every "the notice appears" case is paired with a control over an action that + * declares NO `requiredPermissions`, so deleting the notice outright cannot + * turn a negative pin green for the wrong reason. Case F additionally asserts + * the placement frames still render in the same tree that carries the notice: + * the notice is an addition, and nothing was traded away for it. + * + * ⛔ The end-user side is pinned elsewhere, in the file named above: ruling + * part (a) is that NOTHING changes there, and a notice leaking into a running + * app's list surface is the failure this card must not produce. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, cleanup } from '@testing-library/react'; + +/** + * The capabilities the host reports for the signed-in principal. + * `undefined` is "the host reported nothing", which the gate itself reads as + * unknown and fails OPEN on — so the inspector's first-person clause must stay + * silent rather than guess (case E). + */ +let heldCapabilities: string[] | undefined; + +vi.mock('@object-ui/permissions', async (importOriginal) => { + const actual = await importOriginal>(); + return { + ...actual, + usePermissions: () => ({ + systemPermissions: heldCapabilities, + hasCapabilities: () => true, + check: () => ({ allowed: true }), + checkField: () => true, + getFieldPermissions: () => [], + getRowFilter: () => undefined, + getObjectApiOperations: () => undefined, + roles: [], + userId: null, + isLoaded: true, + can: () => true, + cannot: () => false, + }), + }; +}); + +// ActionDefaultInspector mounts the object/field pickers behind its two +// ConditionBuilders, which call the shared metadata client at mount. Stub it so +// no fetch escapes; same mechanism ActionDefaultInspector.celGate.test.tsx uses. +const state = vi.hoisted(() => ({ + metadataClient: { get: vi.fn(async () => undefined), list: vi.fn(async () => [] as unknown[]) }, +})); +vi.mock('./useMetadata', () => ({ + useMetadataClient: () => state.metadataClient, +})); + +import { ActionDefaultInspector } from './inspectors/ActionDefaultInspector'; +import { ActionPreview } from './previews/ActionPreview'; + +/** The reporting app's action, reduced to the members these cases read. */ +const GATED = { + name: 'duly_catalog_apply_to_people', + label: 'Apply to people', + type: 'script', + objectName: 'duly_catalog_item', + target: 'true', + locations: ['list_toolbar'], + requiredPermissions: ['duly.catalog.apply'], +}; + +/** Same action with the gate removed — the control for every case below. */ +const UNGATED = { ...GATED, requiredPermissions: undefined }; + +function renderInspector(draft: Record) { + return render( + {}} + readOnly={false} + locale="en-US" + />, + ); +} + +function renderPreview(draft: Record) { + return render(); +} + +const gateNote = () => screen.queryByTestId('action-capability-gate-note'); +const selfClause = () => screen.queryByTestId('action-capability-gate-self'); +const previewNote = () => screen.queryByTestId('action-preview-capability-gate-note'); +const previewRequires = () => screen.queryByTestId('action-preview-required-permissions'); + +describe('objectui#7234 — the designer states why a gated action is missing', () => { + beforeEach(() => { + heldCapabilities = undefined; + vi.clearAllMocks(); + }); + afterEach(() => cleanup()); + + it('A: the inspector names the gating capability and says HIDDEN, not disabled', () => { + renderInspector({ ...GATED }); + const note = gateNote(); + expect(note).toBeTruthy(); + expect(note).toHaveTextContent('duly.catalog.apply'); + expect(note?.textContent).toMatch(/hides this action/i); + expect(note?.textContent).toMatch(/Not greyed out and not an\s+error/i); + }); + + it('B (control): an action declaring no requiredPermissions gets no notice', () => { + renderInspector({ ...UNGATED }); + expect(gateNote()).toBeNull(); + }); + + // C and D are ONE differential: identical draft, identical code path, only + // the held capability set differs — the same shape that identified the gate + // as the cause on this card in the first place. + + it('C: a session missing the capability is told so in the first person', () => { + heldCapabilities = ['duly.task.update_status']; + renderInspector({ ...GATED }); + expect(selfClause()).toBeTruthy(); + expect(selfClause()).toHaveTextContent('duly.catalog.apply'); + }); + + it('D: the SAME draft drops that clause once the session holds it', () => { + heldCapabilities = ['duly.catalog.apply']; + renderInspector({ ...GATED }); + // The declaration-side notice stays — the gate still applies to everyone + // else — but nothing claims this session cannot see the button. + expect(gateNote()).toBeTruthy(); + expect(selfClause()).toBeNull(); + }); + + it('E: an unknown held set stays silent instead of guessing (gate fails OPEN)', () => { + heldCapabilities = undefined; + renderInspector({ ...GATED }); + expect(gateNote()).toBeTruthy(); + expect(selfClause()).toBeNull(); + }); + + it('F: the preview carries the capability line AND still draws the placement frames', () => { + renderPreview({ ...GATED }); + expect(previewRequires()).toHaveTextContent('duly.catalog.apply'); + expect(previewNote()).toHaveTextContent('duly.catalog.apply'); + // Paired positive: the notice was added to "Where it appears", it did not + // replace it. The row strip is unique to `PlacementPreview`'s list_toolbar + // frame (the bare location name also appears in the metadata strip), so a + // deleted or emptied PlacementPreview fails here. + expect(screen.getByText('row 1 · row 2 · row 3')).toBeTruthy(); + }); + + it('G (control): the preview says nothing about capabilities for an ungated action', () => { + renderPreview({ ...UNGATED }); + expect(previewRequires()).toBeNull(); + expect(previewNote()).toBeNull(); + expect(screen.getByText('row 1 · row 2 · row 3')).toBeTruthy(); + }); + + it('H: the preview is declaration-side — the held set does not move it', () => { + heldCapabilities = []; + renderPreview({ ...GATED }); + // Positive first: without it both reads are `undefined` and the comparison + // below passes for a deleted notice as readily as for a stable one. + expect(previewNote()).toBeTruthy(); + const withEmptyHeld = previewNote()?.textContent; + cleanup(); + heldCapabilities = ['duly.catalog.apply']; + renderPreview({ ...GATED }); + expect(previewNote()?.textContent).toBe(withEmptyHeld); + }); +}); diff --git a/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx b/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx index ceffbc6a26..96d81acb8e 100644 --- a/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx +++ b/packages/app-shell/src/views/metadata-admin/inspectors/ActionDefaultInspector.tsx @@ -40,6 +40,7 @@ import { import { Button, Label, Textarea, } from '@object-ui/components'; +import { usePermissions } from '@object-ui/permissions'; import type { MetadataDefaultInspectorProps } from '../default-inspector-registry.js'; import { SchemaForm } from '../SchemaForm.js'; import { t } from '../i18n.js'; @@ -440,6 +441,35 @@ export function ActionDefaultInspector({ const params: ActionParam[] = Array.isArray(draft.params) ? (draft.params as ActionParam[]) : []; const locations: string[] = Array.isArray(draft.locations) ? (draft.locations as string[]) : []; + /* ─── objectui#7234 — the OTHER way a correctly-placed action never appears ── + * + * `requiredPermissions` (ADR-0066 D4, `action.requiredPermissions`: "Enforced + * with 403 on the platform action route … and mirrored as a UI hide") filters + * the action out of every ticked surface at once — no button, no greyed-out + * control, no message, no 4xx. The card was opened by an admin who had + * configured the buttons, saw none of them anywhere, and read it as a broken + * feature; nothing in the product said otherwise. + * + * Maintainer ruling 2026-09-08 (option B): end-user behaviour is UNCHANGED — + * the action stays hidden — and the REASON becomes visible where the person + * who configures the app looks. This panel is that channel; the sibling + * notice below it (the empty-`locations` case) is the same idea for the other + * way an action never surfaces. + * + * The held set comes from `MePermissionsProvider` via `usePermissions()` — + * the same signal `useCanAuthorMetadata` reads, NOT a second client-side + * permission derivation. `undefined` there means the host reported nothing + * (no provider, or a backend predating ADR-0066), which the gate itself + * treats as unknown and fails OPEN on, so the "you do not hold it" half stays + * silent rather than guessing. */ + const requiredPermissions: string[] = Array.isArray(draft.requiredPermissions) + ? (draft.requiredPermissions as unknown[]).filter((c): c is string => typeof c === 'string') + : []; + const { systemPermissions: heldCapabilities } = usePermissions(); + const unheldCapabilities = Array.isArray(heldCapabilities) + ? requiredPermissions.filter((c) => !heldCapabilities.includes(c)) + : []; + // AI exposure — flattened keys (objectui/server convention, read by ActionPreview). const aiExposed = draft.aiExposed === true; const aiDescription = typeof draft.aiDescription === 'string' ? (draft.aiDescription as string) : ''; @@ -636,6 +666,31 @@ export function ActionDefaultInspector({ placement above, or place it from a view’s bulk actions. )} + {/* [#7234] The capability gate, stated where the person configuring the + app looks. Deliberately NOT an error: the hide is correct behaviour + under ADR-0066 D4 and the maintainer ruled it stays. What was + missing is that it was the one kind of disappearance nothing + anywhere explained. See the decode block above for the ruling. */} + {requiredPermissions.length > 0 && ( +
+ Capability-gated on{' '} + {requiredPermissions.join(' + ')} — every placement + ticked above hides this action from anyone who does not hold{' '} + {requiredPermissions.length > 1 ? 'all of them' : 'it'}. Not greyed out and not an + error: the button is simply absent, and the server refuses the invocation with a 403 + either way. + {unheldCapabilities.length > 0 && ( + <> + {' '} + + This session does not hold{' '} + {unheldCapabilities.join(' + ')}, so the button + is hidden from you too — grant it through a permission set to see it in the app. + + + )} +
+ )}
onPatch({ component: v })} disabled={readOnly} />
diff --git a/packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx b/packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx index 1af32b4889..bcdc30fa97 100644 --- a/packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx +++ b/packages/app-shell/src/views/metadata-admin/previews/ActionPreview.tsx @@ -10,8 +10,9 @@ * and `label` so authors can see the visual weight before they * ship it (primary buttons are highlighted, danger turns red, * icon-only actions render a compact icon button). - * 2. A metadata strip: type, target, locations - * flag, AI exposure, refreshAfter, confirmText. + * 2. A metadata strip: type, target, locations, the + * `requiredPermissions` capability gate, AI exposure, + * refreshAfter, confirmText. * 3. A params table when the action prompts the user — this is the * modal/drawer it would open on click. We render it as a static * preview, not an interactive form, because previews must be @@ -160,6 +161,19 @@ export function ActionPreview({ name, draft }: MetadataPreviewProps) { const variant = (d.variant as string | undefined) || undefined; const component = String(d.component ?? ''); const locations = Array.isArray(d.locations) ? (d.locations as string[]) : []; + /* [objectui#7234] `requiredPermissions` (ADR-0066 D4) is the reason a + correctly-placed action can be absent from every surface at once, with no + error and no message. Until this line the preview was silent about it while + "Where it appears" below drew the button in every declared frame — the + exact shape the `global_nav` note in `PlacementPreview` rules against: a + designer promising a surface the running app does not draw. Maintainer + ruling 2026-09-08 keeps the hide for end users and puts the reason here, + where the author looks. Declaration-side only — whether THIS session holds + the capability is answered in the inspector beside this preview, which + reads the live held set. */ + const requiredPermissions = Array.isArray(d.requiredPermissions) + ? (d.requiredPermissions as unknown[]).filter((c): c is string => typeof c === 'string') + : []; // No `shortcut` / `bulkEnabled` here: both are spec-17 `retiredKey()` // tombstones, so a preview of them could only ever render for metadata the // platform now refuses to parse. See ActionDefaultInspector's RETIRED_FIELDS. @@ -221,6 +235,20 @@ export function ActionPreview({ name, draft }: MetadataPreviewProps) { ))} )} + {requiredPermissions.length > 0 && ( +
+ + Requires: + {requiredPermissions.map((c) => ( + + {c} + + ))} +
+ )} {Boolean(typeof visible === 'string' || (visible && typeof visible === 'object')) && ( )} @@ -254,6 +282,13 @@ export function ActionPreview({ name, draft }: MetadataPreviewProps) { {/* Placement simulation — where this action surfaces */} {locations.length > 0 && (
+ {requiredPermissions.length > 0 && ( +
+ Hidden from anyone who does not hold{' '} + {requiredPermissions.join(' + ')} — the frames + below show the placement, not who can see it. +
+ )}
)}