Skip to content

Commit 97e7e3c

Browse files
Jack Qclaude
andauthored
feat(spec): unify ActionSchema.visible/disabled on one condition shape (#6450)
* feat(spec): unify ActionSchema.visible / disabled on one condition shape (#5970) Both keys now accept `boolean | string(CEL) | {dialect, source}`. `visible` was missing the boolean arm, so `visible: true` was a spec-side parse error while objectui's ActionDef accepted it — an asymmetry between two keys of the same kind is a dialect nursery (Prime Directive #12), and it is what forced console's `(action as any).disabled` cast. The boolean arm is not normalized into a CEL literal: a literal survives as a literal so a renderer can branch on it without an evaluator. `requiresFeature` lowering now meets two literals it never could before: `visible: true` composes to the gate alone (`true && gate` IS `gate`), while `visible: false` is refused loudly — the gate could never fire, which is the parses-clean-changes-nothing shape ADR-0078 rejects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G3U9PJm1hEJitS9LtZz8TC * spec: regenerate reference docs for the unified condition shape (#5970) `gen:docs` — action.mdx now prints one shape for both keys (`boolean | string | {dialect,...}`), and bulk-action.mdx carries the reworded `visible` that no longer claims shape-identity with `action.visible`. Also fixes two type errors the new tests introduced, rather than re-recording them: the `getMetadataTypeSchema('action')` call follows the file's own defined-then-assert precedent, and `lowerRequiresFeature` annotates its `visible` local because a generic `Omit<T, …>` indexed access cannot be narrowed by control flow — without it the `=== true` / `=== false` guards leave the boolean arm on the type and the envelope spread does not compile. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G3U9PJm1hEJitS9LtZz8TC --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f1850d8 commit 97e7e3c

7 files changed

Lines changed: 259 additions & 10 deletions

File tree

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

‎content/docs/references/ui/action.mdx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,9 @@ const result = ActionSchema.parse(data);
8080
| **refreshAfter** | `boolean` | optional | Refresh view after execution |
8181
| **undoable** | `boolean` | optional | Offer an Undo affordance after this single-record update action succeeds. |
8282
| **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). |
83-
| **visible** | `string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Visibility predicate (CEL). |
83+
| **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. |
8484
| **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. |
85-
| **disabled** | `boolean \| string \| { dialect: Enum<'cel' \| 'cron' \| 'template'>; source?: string; ast?: any; meta?: object }` | optional | Boolean or predicate (CEL) — action is disabled when TRUE. |
85+
| **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. |
8686
| **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. |
8787
| **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. |
8888
| **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‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ const result = BulkActionDefSchema.parse(data);
4949
| **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. |
5050
| **confirmText** | `string` | optional | Confirmation text shown above the affected-record summary. |
5151
| **confirmLabel** | `string` | optional | Custom Confirm button label (default: "Run"). |
52-
| **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. |
52+
| **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. |
5353
| **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. |
5454
| **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. |
5555
| **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‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,12 @@ export function featureGatePredicate(name: PublicAuthFeatureName): string {
281281
/** Object shape the lowering transform operates on (post field-level parse). */
282282
type WithRequiresFeature = {
283283
requiresFeature?: PublicAuthFeatureName;
284-
/** Already normalized by ExpressionInputSchema to the `{dialect, source}` envelope. */
285-
visible?: { dialect?: unknown; source?: unknown } & Record<string, unknown>;
284+
/**
285+
* Already normalized by ExpressionInputSchema to the `{dialect, source}`
286+
* envelope — except for the literal arm, which surfaces here verbatim on the
287+
* surfaces that declare one (`ActionSchema.visible`, #5970).
288+
*/
289+
visible?: boolean | ({ dialect?: unknown; source?: unknown } & Record<string, unknown>);
286290
};
287291

288292
/**
@@ -296,6 +300,14 @@ type WithRequiresFeature = {
296300
* - Existing CEL `visible` with a `source` → composed as
297301
* `(<existing>) && <gate>` (existing predicate first, gate last — the
298302
* hand-written convention).
303+
* - Existing `visible: true` → the gate alone. `true && <gate>` IS `<gate>`, so
304+
* an author who spelled the default out explicitly gets the same lowering as
305+
* one who omitted the key (the literal arm arrived with #5970).
306+
* - Existing `visible: false` → loud parse error. Here the boolean algebra runs
307+
* the other way: `false && <gate>` is `false` whatever the flag says, so the
308+
* gate could never take effect and the declaration is inert on arrival —
309+
* precisely the parses-clean-changes-nothing key ADR-0078 exists to reject.
310+
* Drop one of the two rather than shipping a gate that reads as load-bearing.
299311
* - Existing `visible` that is non-CEL or AST-only → loud parse error
300312
* (ADR-0078 no-silently-inert); write the combined predicate by hand.
301313
*
@@ -310,10 +322,27 @@ export function lowerRequiresFeature<T extends WithRequiresFeature>(
310322
if (requiresFeature === undefined) return rest as Omit<T, 'requiresFeature'>;
311323

312324
const gate = featureGatePredicate(requiresFeature);
313-
const existing = rest.visible;
314-
if (existing === undefined) {
325+
// Annotated rather than inferred: `rest` is a generic `Omit<T, …>`, so
326+
// `rest.visible` is a deferred indexed access that control flow cannot narrow
327+
// — the `=== true` / `=== false` guards below would not strip the boolean arm
328+
// off it, and the envelope spread at the end would not compile.
329+
const existing: WithRequiresFeature['visible'] = rest.visible;
330+
// `true` is the explicit spelling of "no gate of my own" — same lowering as an
331+
// absent key. `false` can never be gated into visibility, so it is refused.
332+
if (existing === undefined || existing === true) {
315333
return { ...rest, visible: { dialect: 'cel', source: gate } } as Omit<T, 'requiresFeature'>;
316334
}
335+
if (existing === false) {
336+
ctx.addIssue({
337+
code: 'custom',
338+
path: ['requiresFeature'],
339+
message:
340+
'`requiresFeature` cannot compose with `visible: false` — the literal already hides this ' +
341+
'unconditionally, so the feature gate can never take effect. Drop `requiresFeature` to keep it ' +
342+
'hidden, or drop `visible: false` to let the flag decide.',
343+
});
344+
return rest as Omit<T, 'requiresFeature'>;
345+
}
317346
if (existing.dialect !== 'cel' || typeof existing.source !== 'string') {
318347
ctx.addIssue({
319348
code: 'custom',

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

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { describe, it, expect } from 'vitest';
2+
import { z } from 'zod';
23
import { ActionSchema, ActionParamSchema, Action, type Action as ActionType, ACTION_LOCATIONS, ActionLocationSchema, type ActionLocation } from './action.zod';
34
import { getMetadataTypeSchema } from '../kernel/metadata-type-schemas';
45
import { ObjectSchema } from '../data/object.zod';
@@ -252,6 +253,126 @@ describe('requiresFeature lowering', () => {
252253
expect(bare.visible).toBeUndefined();
253254
expect(bare).not.toHaveProperty('requiresFeature');
254255
});
256+
257+
// #5970 gave `visible` a boolean arm, so the lowering now meets two literals
258+
// it never could before. Boolean algebra decides both, in opposite directions.
259+
it('treats `visible: true` as the explicit default — lowers to the gate alone', () => {
260+
const result = ActionSchema.parse({
261+
name: 'invite_user',
262+
label: 'Invite',
263+
type: 'api',
264+
target: '/api/v1/auth/organization/invite-member',
265+
visible: true,
266+
requiresFeature: 'organization',
267+
} satisfies z.input<typeof ActionSchema>);
268+
// Identical to the sugar-only case above: `true && <gate>` IS `<gate>`.
269+
expect(result.visible).toEqual({ dialect: 'cel', source: 'features.organization != false' });
270+
expect(result).not.toHaveProperty('requiresFeature');
271+
});
272+
273+
it('rejects composition with `visible: false` loudly — the gate could never fire (ADR-0078)', () => {
274+
const result = ActionSchema.safeParse({
275+
name: 'invite_user',
276+
label: 'Invite',
277+
type: 'api',
278+
target: '/api/v1/auth/organization/invite-member',
279+
visible: false,
280+
requiresFeature: 'organization',
281+
} satisfies z.input<typeof ActionSchema>);
282+
expect(result.success).toBe(false);
283+
if (!result.success) {
284+
const issue = result.error.issues.find((i) => i.path.includes('requiresFeature'));
285+
expect(issue).toBeDefined();
286+
// The message must name BOTH exits, not just the diagnosis.
287+
expect(issue!.message).toContain('`visible: false`');
288+
expect(issue!.message).toContain('Drop `requiresFeature`');
289+
}
290+
});
291+
});
292+
293+
// ── #5970 — `visible` / `disabled` speak ONE shape ────────────────────────────
294+
// Ruled 2026-08-06: both keys are `boolean | string(CEL) | {dialect, source}`.
295+
// Before this, `visible` had no boolean arm while `disabled` did, so the very
296+
// common `visible: true` was a spec-side parse error that objectui's `ActionDef`
297+
// accepted anyway — see the `ActionConditionInputSchema` comment in
298+
// `action.zod.ts` for why an asymmetry between two keys of the same kind is a
299+
// dialect nursery rather than a cosmetic gap.
300+
describe('ActionSchema — visible/disabled unified condition shape', () => {
301+
const base = {
302+
name: 'transfer_ownership',
303+
label: 'Transfer Ownership',
304+
type: 'api',
305+
target: '/api/v1/auth/organization/update-member-role',
306+
} as const satisfies Partial<z.input<typeof ActionSchema>>;
307+
308+
const parse = (key: 'visible' | 'disabled', value: unknown) =>
309+
ActionSchema.safeParse({ ...base, [key]: value });
310+
311+
for (const key of ['visible', 'disabled'] as const) {
312+
describe(`\`${key}\``, () => {
313+
it('accepts the boolean arm and keeps the literal a literal', () => {
314+
// NOT normalized into `{dialect:'cel', source:'true'}`: a renderer must
315+
// be able to branch on the constant without standing up an evaluator.
316+
for (const literal of [true, false]) {
317+
const result = parse(key, literal);
318+
expect(result.success, `${key}: ${literal}`).toBe(true);
319+
if (result.success) expect(result.data[key]).toBe(literal);
320+
}
321+
});
322+
323+
it('accepts the CEL string arm and normalizes it to the envelope', () => {
324+
const result = parse(key, "record.status == 'open'");
325+
expect(result.success).toBe(true);
326+
if (result.success) {
327+
expect(result.data[key]).toEqual({ dialect: 'cel', source: "record.status == 'open'" });
328+
}
329+
});
330+
331+
it('accepts the full envelope arm verbatim, authorship metadata included', () => {
332+
const envelope = {
333+
dialect: 'cel' as const,
334+
source: "record.status == 'open'",
335+
meta: { rationale: 'only open records can transfer', generatedBy: 'agent:spec' },
336+
};
337+
const result = parse(key, envelope);
338+
expect(result.success).toBe(true);
339+
if (result.success) expect(result.data[key]).toEqual(envelope);
340+
});
341+
342+
// The widening must not shrink the rejection surface by one shape. Each
343+
// of these was rejected before #5970 and is asserted to still be.
344+
it.each([
345+
['an empty CEL string', ''],
346+
['a number', 1],
347+
['null', null],
348+
['an envelope with neither `source` nor `ast`', { dialect: 'cel' }],
349+
['an envelope missing `dialect`', { source: "record.status == 'open'" }],
350+
['an envelope with an unknown dialect', { dialect: 'sql', source: 'SELECT 1' }],
351+
['a bare empty object', {}],
352+
['an array of predicates', ["record.a == 1", "record.b == 2"]],
353+
])('still rejects %s', (_label, value) => {
354+
expect(parse(key, value).success).toBe(false);
355+
});
356+
});
357+
}
358+
359+
it('accepts both keys on one action, each on a different arm', () => {
360+
const result = ActionSchema.parse({
361+
...base,
362+
visible: true,
363+
disabled: "record.status == 'closed'",
364+
} satisfies z.input<typeof ActionSchema>);
365+
expect(result.visible).toBe(true);
366+
expect(result.disabled).toEqual({ dialect: 'cel', source: "record.status == 'closed'" });
367+
});
368+
369+
it('reaches the same shape through the registered `action` metadata schema', () => {
370+
// The authoring door the Studio form and `GET /api/v1/meta` go through —
371+
// a widening that stopped at the bare export would not reach an author.
372+
const schema = getMetadataTypeSchema('action');
373+
expect(schema, "the 'action' metadata type must resolve to a schema").toBeDefined();
374+
expect(schema!.safeParse({ ...base, visible: true, disabled: false }).success).toBe(true);
375+
});
255376
});
256377

257378
describe('ActionSchema', () => {

0 commit comments

Comments
 (0)