feat(spec): unify ActionSchema.visible/disabled on one condition shape

.changeset/action-visible-disabled-unified-condition.md

@@ -0,0 +1,57 @@
+---
+"@objectstack/spec": minor
+---
+
+feat(spec): `ActionSchema.visible` / `disabled` speak one shape — `boolean | string(CEL) | {dialect, source}` (#5970)
+
+An action's two condition keys accepted different vocabularies. `disabled` took
+all three arms; `visible` had no **boolean** arm, so `visible: true` — the most
+obvious thing an author can write, and a shape already present in stored
+metadata — was a parse error on the spec side while objectui's `ActionDef`
+accepted it and pinned it with tests.
+
+Both keys now accept the same three arms, cheapest first:
+
+| arm | example | meaning |
+|:---|:---|:---|
+| `boolean` | `visible: false` | the degenerate literal — settled at authoring time |
+| `string` | `disabled: "record.status == 'closed'"` | CEL shorthand, normalized to the envelope at parse time |
+| `{ dialect, source }` | `{ dialect: 'cel', source: '…', meta: { rationale } }` | the full envelope, for authorship metadata or a non-default dialect |
+
+**Purely additive** — every shape that parsed before parses the same way, and
+every shape that was rejected is still rejected (an empty CEL string, a number,
+`null`, an envelope missing `dialect`, an envelope with neither `source` nor
+`ast`, an unknown dialect). No migration, no ADR-0087 disposition: nothing an
+author can write was removed or renamed.
+
+The boolean arm is deliberately **not** normalized into
+`{dialect: 'cel', source: 'true'}`. A literal survives as a literal so a
+renderer can branch on it without standing up an evaluator, and `false` stays
+statically greppable.
+
+**Why unify rather than leave it.** An asymmetry between two keys that mean the
+same *kind* of thing is a dialect nursery: it teaches every consumer to carry
+its own widening, and each of those is a second de-facto contract (Prime
+Directive #12). Console's `DeclaredActionsBar` was carrying exactly that as an
+`(action as any).disabled` cast. This change is what lets #4075 step 3 derive
+objectui's `ActionDef` from the spec schema and delete the casts.
+
+**One new rejection, at the interaction with `requiresFeature`.** The
+declarative feature-gate sugar lowers into `visible`, so it now meets two
+literals it never could before, and boolean algebra decides them in opposite
+directions:
+
+- `visible: true` + `requiresFeature: 'x'` → the gate alone. `true && <gate>` IS
+  `<gate>`, so spelling the default out explicitly lowers exactly like omitting
+  the key.
+- `visible: false` + `requiresFeature: 'x'` → **parse error**. `false && <gate>`
+  is `false` whatever the flag says, so the gate could never take effect and the
+  declaration is inert on arrival — the parses-clean-changes-nothing shape
+  ADR-0078 exists to reject. The message names both exits: drop
+  `requiresFeature` to keep it hidden, or drop `visible: false` to let the flag
+  decide. This combination was unwritable before (the boolean arm did not
+  exist), so no stored metadata can carry it.
+
+`bulkActions[].visible` is unchanged and keeps the two predicate arms only — a
+per-record eligibility predicate has nothing to say as a constant. Its
+description no longer claims shape-identity with `action.visible`.

content/docs/references/ui/action.mdx

@@ -80,9 +80,9 @@ const result = ActionSchema.parse(data);
 | **refreshAfter** | `boolean` | optional | Refresh view after execution |
 | **undoable** | `boolean` | optional | Offer an Undo affordance after this single-record update action succeeds. |
 | **resultDialog** | `{ title?: string; description?: string; acknowledge?: string; format?: Enum<'qrcode' \| 'code-list' \| 'secret' \| 'text' \| 'json'>; … }` | optional | Render API response in a one-shot reveal dialog (suppresses successMessage when set). |
-| **visible** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Visibility predicate (CEL). |
+| **visible** | `boolean \| string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Visibility predicate — `true`/`false` literal, CEL string, or `{dialect, source}` envelope. The action is offered when it evaluates TRUE. Omit = always visible. |
 | **requiresFeature** | `Enum<'twoFactor' \| 'passkeys' \| 'magicLink' \| 'organization' \| 'multiOrgEnabled' \| 'degradedTenancy' \| 'oidcProvider' \| 'sso' \| 'ssoEnforced' \| 'deviceAuthorization' \| … +3 more>` | optional | Public auth feature flag gating this action; lowered into `visible` at parse time. |
-| **disabled** | `boolean \| string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Boolean or predicate (CEL) — action is disabled when TRUE. |
+| **disabled** | `boolean \| string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Disabled predicate — `true`/`false` literal, CEL string, or `{dialect, source}` envelope. The action is shown but refused when it evaluates TRUE. Omit = never disabled. |
 | **requiredPermissions** | `string[]` | optional | [ADR-0066 D4] Capabilities required to invoke this action. Enforced with 403 on the platform action route (script/flow/modal + MCP) and mirrored as a UI hide; a `type: api` action pointed at a custom endpoint must re-check it there. |
 | **shortcut** | `never` | optional | [REMOVED] `action.shortcut` was removed in @objectstack/spec 17.0.0 (#3896 audit close-out) — it never triggered anything: no keydown listener feeds ActionEngine.getShortcuts(), and objectui's keyboard stack (useKeyboardShortcuts) is hand-registered and never consults action metadata. Delete the key. For a real shortcut, register the key in the Console keyboard stack and have its handler invoke the action by name. |
 | **bulkEnabled** | `never` | optional | [REMOVED] `action.bulkEnabled` was removed in @objectstack/spec 17.0.0 (#3896 audit close-out) — the multi-select toolbar is driven by the LIST VIEW's `bulkActions` / `bulkActionDefs`, never by this flag, so setting it changed nothing. Delete the key and declare the action in the view's `bulkActions` instead. |

content/docs/references/ui/bulk-action.mdx

@@ -49,7 +49,7 @@ const result = BulkActionDefSchema.parse(data);
 | **params** | `({ name: string; label?: string; help?: string; type: Enum<'text' \| 'textarea' \| 'email' \| 'url' \| 'phone' \| 'password' \| 'secret' \| … +42 more>; … } & Record<string, any>)[]` | optional | Inputs collected once before the run. Omit to skip the params step and go straight to confirm. |
 | **confirmText** | `string` | optional | Confirmation text shown above the affected-record summary. |
 | **confirmLabel** | `string` | optional | Custom Confirm button label (default: "Run"). |
-| **visible** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Eligibility predicate (CEL), same shape as `action.visible`. Evaluated once PER SELECTED RECORD with that record bound: the button is offered when at least one passes, the run covers only those, and the rest are reported as skipped. A record-free predicate (`features.x`, `current_user.y`) therefore behaves as a plain button-level gate. Fail-closed — a predicate that faults excludes the record. |
+| **visible** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Eligibility predicate (CEL) — a string or a `{dialect, source}` envelope, i.e. `action.visible` without its boolean-literal arm (#5970): a per-record predicate has nothing to say as a constant. Evaluated once PER SELECTED RECORD with that record bound: the button is offered when at least one passes, the run covers only those, and the rest are reported as skipped. A record-free predicate (`features.x`, `current_user.y`) therefore behaves as a plain button-level gate. Fail-closed — a predicate that faults excludes the record. |
 | **requiredPermissions** | `string[]` | optional | [ADR-0066 D4] Capability gate on the button, `action.requiredPermissions` semantics verbatim: absent or empty always passes, several are AND-ed, and a client that cannot resolve the caller's capabilities fails OPEN (the server stays the authority). This key exists for INLINE defs — notably the `update`/`delete` data-plane forms, which dispatch no action and so have nothing to inherit a gate from; a def promoted from `bulkActions: ['<name>']` (or an aggregate def naming a declared action) inherits the action's own declaration instead. On a data-plane def the gate governs visibility only — the write itself is still authorized by the data API's object permissions and server hooks. |
 | **maxRecords** | `integer` | optional | Selection size above which the run is blocked. Set it on defs whose server work is expensive — an aggregate def carries every selected id in one request. |
 | **batchSize** | `integer` | optional | Records per executor batch (default 200). Data-plane operations only — an aggregate run is a single call by definition. |

packages/spec/src/kernel/public-auth-features.ts

@@ -281,8 +281,12 @@ export function featureGatePredicate(name: PublicAuthFeatureName): string {
 /** Object shape the lowering transform operates on (post field-level parse). */
 type WithRequiresFeature = {
   requiresFeature?: PublicAuthFeatureName;
-  /** Already normalized by ExpressionInputSchema to the `{dialect, source}` envelope. */
-  visible?: { dialect?: unknown; source?: unknown } & Record<string, unknown>;
+  /**
+   * Already normalized by ExpressionInputSchema to the `{dialect, source}`
+   * envelope — except for the literal arm, which surfaces here verbatim on the
+   * surfaces that declare one (`ActionSchema.visible`, #5970).
+   */
+  visible?: boolean | ({ dialect?: unknown; source?: unknown } & Record<string, unknown>);
 };
 
 /**
@@ -296,6 +300,14 @@ type WithRequiresFeature = {
  * - Existing CEL `visible` with a `source` → composed as
  *   `(<existing>) && <gate>` (existing predicate first, gate last — the
  *   hand-written convention).
+ * - Existing `visible: true` → the gate alone. `true && <gate>` IS `<gate>`, so
+ *   an author who spelled the default out explicitly gets the same lowering as
+ *   one who omitted the key (the literal arm arrived with #5970).
+ * - Existing `visible: false` → loud parse error. Here the boolean algebra runs
+ *   the other way: `false && <gate>` is `false` whatever the flag says, so the
+ *   gate could never take effect and the declaration is inert on arrival —
+ *   precisely the parses-clean-changes-nothing key ADR-0078 exists to reject.
+ *   Drop one of the two rather than shipping a gate that reads as load-bearing.
  * - Existing `visible` that is non-CEL or AST-only → loud parse error
  *   (ADR-0078 no-silently-inert); write the combined predicate by hand.
  *
@@ -310,10 +322,27 @@ export function lowerRequiresFeature<T extends WithRequiresFeature>(
   if (requiresFeature === undefined) return rest as Omit<T, 'requiresFeature'>;
 
   const gate = featureGatePredicate(requiresFeature);
-  const existing = rest.visible;
-  if (existing === undefined) {
+  // Annotated rather than inferred: `rest` is a generic `Omit<T, …>`, so
+  // `rest.visible` is a deferred indexed access that control flow cannot narrow
+  // — the `=== true` / `=== false` guards below would not strip the boolean arm
+  // off it, and the envelope spread at the end would not compile.
+  const existing: WithRequiresFeature['visible'] = rest.visible;
+  // `true` is the explicit spelling of "no gate of my own" — same lowering as an
+  // absent key. `false` can never be gated into visibility, so it is refused.
+  if (existing === undefined || existing === true) {
     return { ...rest, visible: { dialect: 'cel', source: gate } } as Omit<T, 'requiresFeature'>;
   }
+  if (existing === false) {
+    ctx.addIssue({
+      code: 'custom',
+      path: ['requiresFeature'],
+      message:
+        '`requiresFeature` cannot compose with `visible: false` — the literal already hides this ' +
+        'unconditionally, so the feature gate can never take effect. Drop `requiresFeature` to keep it ' +
+        'hidden, or drop `visible: false` to let the flag decide.',
+    });
+    return rest as Omit<T, 'requiresFeature'>;
+  }
   if (existing.dialect !== 'cel' || typeof existing.source !== 'string') {
     ctx.addIssue({
       code: 'custom',

packages/spec/src/ui/action.test.ts

@@ -1,4 +1,5 @@
 import { describe, it, expect } from 'vitest';
+import { z } from 'zod';
 import { ActionSchema, ActionParamSchema, Action, type Action as ActionType, ACTION_LOCATIONS, ActionLocationSchema, type ActionLocation } from './action.zod';
 import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas';
 import { ObjectSchema } from '../data/object.zod';
@@ -252,6 +253,126 @@ describe('requiresFeature lowering', () => {
     expect(bare.visible).toBeUndefined();
     expect(bare).not.toHaveProperty('requiresFeature');
   });
+
+  // #5970 gave `visible` a boolean arm, so the lowering now meets two literals
+  // it never could before. Boolean algebra decides both, in opposite directions.
+  it('treats `visible: true` as the explicit default — lowers to the gate alone', () => {
+    const result = ActionSchema.parse({
+      name: 'invite_user',
+      label: 'Invite',
+      type: 'api',
+      target: '/api/v1/auth/organization/invite-member',
+      visible: true,
+      requiresFeature: 'organization',
+    } satisfies z.input<typeof ActionSchema>);
+    // Identical to the sugar-only case above: `true && <gate>` IS `<gate>`.
+    expect(result.visible).toEqual({ dialect: 'cel', source: 'features.organization != false' });
+    expect(result).not.toHaveProperty('requiresFeature');
+  });
+
+  it('rejects composition with `visible: false` loudly — the gate could never fire (ADR-0078)', () => {
+    const result = ActionSchema.safeParse({
+      name: 'invite_user',
+      label: 'Invite',
+      type: 'api',
+      target: '/api/v1/auth/organization/invite-member',
+      visible: false,
+      requiresFeature: 'organization',
+    } satisfies z.input<typeof ActionSchema>);
+    expect(result.success).toBe(false);
+    if (!result.success) {
+      const issue = result.error.issues.find((i) => i.path.includes('requiresFeature'));
+      expect(issue).toBeDefined();
+      // The message must name BOTH exits, not just the diagnosis.
+      expect(issue!.message).toContain('`visible: false`');
+      expect(issue!.message).toContain('Drop `requiresFeature`');
+    }
+  });
+});
+
+// ── #5970 — `visible` / `disabled` speak ONE shape ────────────────────────────
+// Ruled 2026-08-06: both keys are `boolean | string(CEL) | {dialect, source}`.
+// Before this, `visible` had no boolean arm while `disabled` did, so the very
+// common `visible: true` was a spec-side parse error that objectui's `ActionDef`
+// accepted anyway — see the `ActionConditionInputSchema` comment in
+// `action.zod.ts` for why an asymmetry between two keys of the same kind is a
+// dialect nursery rather than a cosmetic gap.
+describe('ActionSchema — visible/disabled unified condition shape', () => {
+  const base = {
+    name: 'transfer_ownership',
+    label: 'Transfer Ownership',
+    type: 'api',
+    target: '/api/v1/auth/organization/update-member-role',
+  } as const satisfies Partial<z.input<typeof ActionSchema>>;
+
+  const parse = (key: 'visible' | 'disabled', value: unknown) =>
+    ActionSchema.safeParse({ ...base, [key]: value });
+
+  for (const key of ['visible', 'disabled'] as const) {
+    describe(`\`${key}\``, () => {
+      it('accepts the boolean arm and keeps the literal a literal', () => {
+        // NOT normalized into `{dialect:'cel', source:'true'}`: a renderer must
+        // be able to branch on the constant without standing up an evaluator.
+        for (const literal of [true, false]) {
+          const result = parse(key, literal);
+          expect(result.success, `${key}: ${literal}`).toBe(true);
+          if (result.success) expect(result.data[key]).toBe(literal);
+        }
+      });
+
+      it('accepts the CEL string arm and normalizes it to the envelope', () => {
+        const result = parse(key, "record.status == 'open'");
+        expect(result.success).toBe(true);
+        if (result.success) {
+          expect(result.data[key]).toEqual({ dialect: 'cel', source: "record.status == 'open'" });
+        }
+      });
+
+      it('accepts the full envelope arm verbatim, authorship metadata included', () => {
+        const envelope = {
+          dialect: 'cel' as const,
+          source: "record.status == 'open'",
+          meta: { rationale: 'only open records can transfer', generatedBy: 'agent:spec' },
+        };
+        const result = parse(key, envelope);
+        expect(result.success).toBe(true);
+        if (result.success) expect(result.data[key]).toEqual(envelope);
+      });
+
+      // The widening must not shrink the rejection surface by one shape. Each
+      // of these was rejected before #5970 and is asserted to still be.
+      it.each([
+        ['an empty CEL string', ''],
+        ['a number', 1],
+        ['null', null],
+        ['an envelope with neither `source` nor `ast`', { dialect: 'cel' }],
+        ['an envelope missing `dialect`', { source: "record.status == 'open'" }],
+        ['an envelope with an unknown dialect', { dialect: 'sql', source: 'SELECT 1' }],
+        ['a bare empty object', {}],
+        ['an array of predicates', ["record.a == 1", "record.b == 2"]],
+      ])('still rejects %s', (_label, value) => {
+        expect(parse(key, value).success).toBe(false);
+      });
+    });
+  }
+
+  it('accepts both keys on one action, each on a different arm', () => {
+    const result = ActionSchema.parse({
+      ...base,
+      visible: true,
+      disabled: "record.status == 'closed'",
+    } satisfies z.input<typeof ActionSchema>);
+    expect(result.visible).toBe(true);
+    expect(result.disabled).toEqual({ dialect: 'cel', source: "record.status == 'closed'" });
+  });
+
+  it('reaches the same shape through the registered `action` metadata schema', () => {
+    // The authoring door the Studio form and `GET /api/v1/meta` go through —
+    // a widening that stopped at the bare export would not reach an author.
+    const schema = getMetadataTypeSchema('action');
+    expect(schema, "the 'action' metadata type must resolve to a schema").toBeDefined();
+    expect(schema!.safeParse({ ...base, visible: true, disabled: false }).success).toBe(true);
+  });
 });
 
 describe('ActionSchema', () => {