diff --git a/.changeset/validate-action-predicates.md b/.changeset/validate-action-predicates.md new file mode 100644 index 0000000000..cb12e5f6be --- /dev/null +++ b/.changeset/validate-action-predicates.md @@ -0,0 +1,22 @@ +--- +"@objectstack/cli": minor +"@objectstack/formula": patch +--- + +build: validate UI action `visible` / `disabled` predicates at compile time + +Extends the ADR-0032 build-time expression check to cover action `visible` and +`disabled` predicates (stack-level and object-attached), evaluated record-scoped +like validation rules. A record-header / row action's `visible` is evaluated by +`ActionEngine` against `{ record, recordId, objectName, user, … }` with +fail-closed semantics, so a **bare** field reference (`!done` instead of +`!record.done`) throws at runtime and the action is **silently hidden on every +record** — the trap behind the #2183 "Mark Done never hides" debugging hunt. +`os build` now reports it as an error with the corrective `record.` +message instead of letting it ship. + +`@objectstack/formula`: `ctx` and `features` are added to the record-scope +namespace roots (alongside the existing `user`, `data`, `context`, …) so the +ambient globals real action predicates use (`record.id == ctx.user.id`, +`features.multiOrgEnabled`) are not false-positives. Verified against the full +monorepo build (every example + platform bundle still compiles clean). diff --git a/packages/cli/src/utils/validate-expressions.test.ts b/packages/cli/src/utils/validate-expressions.test.ts index 6536487a6e..3eb8d35457 100644 --- a/packages/cli/src/utils/validate-expressions.test.ts +++ b/packages/cli/src/utils/validate-expressions.test.ts @@ -233,4 +233,58 @@ describe('validateStackExpressions (ADR-0032 build-time)', () => { expect(issues[0].severity).toBe('error'); }); }); + + describe('action visible/disabled predicates (record-scoped) — #2183 class', () => { + it('flags a bare-field `visible` on a stack action (the trap that hid Mark Done)', () => { + const issues = validateStackExpressions({ + objects: [{ name: 'showcase_task', fields: { done: { type: 'boolean' }, status: { type: 'select' } } }], + actions: [{ name: 'mark_done', objectName: 'showcase_task', type: 'script', locations: ['record_header'], visible: '!done' }], + }); + const v = issues.filter(i => i.where.includes("action 'mark_done' visible")); + expect(v).toHaveLength(1); + expect(v[0].severity).toBe('error'); + expect(v[0].message).toMatch(/bare reference `done`/); + }); + + it('accepts the record-qualified form', () => { + const issues = validateStackExpressions({ + objects: [{ name: 'showcase_task', fields: { done: { type: 'boolean' } } }], + actions: [{ name: 'mark_done', objectName: 'showcase_task', type: 'script', visible: '!record.done' }], + }); + expect(issues).toHaveLength(0); + }); + + it('accepts ambient globals (ctx / features / user) used by platform actions', () => { + const issues = validateStackExpressions({ + objects: [{ name: 'sys_user', fields: { id: { type: 'text' }, email_verified: { type: 'boolean' } } }], + actions: [{ name: 'verify_email', objectName: 'sys_user', visible: 'record.id == ctx.user.id && record.email_verified == false && features.x != true' }], + }); + expect(issues).toHaveLength(0); + }); + + it('flags a bare-field `disabled` predicate but ignores a boolean `disabled`', () => { + const bad = validateStackExpressions({ + objects: [{ name: 'crm_lead', fields: { status: { type: 'select' } } }], + actions: [{ name: 'park', objectName: 'crm_lead', disabled: 'status == "converted"' }], + }); + expect(bad.filter(i => i.where.includes("action 'park' disabled"))).toHaveLength(1); + + const ok = validateStackExpressions({ + objects: [{ name: 'crm_lead', fields: { status: { type: 'select' } } }], + actions: [{ name: 'park', objectName: 'crm_lead', disabled: true }], + }); + expect(ok).toHaveLength(0); + }); + + it('validates an action attached to an object (record scope = parent object)', () => { + const issues = validateStackExpressions({ + objects: [{ + name: 'showcase_task', + fields: { done: { type: 'boolean' } }, + actions: [{ name: 'mark_done', type: 'script', visible: '!done' }], + }], + }); + expect(issues.filter(i => i.where.includes("action 'mark_done' visible"))).toHaveLength(1); + }); + }); }); diff --git a/packages/cli/src/utils/validate-expressions.ts b/packages/cli/src/utils/validate-expressions.ts index 63f0d271dd..8d1c450b4e 100644 --- a/packages/cli/src/utils/validate-expressions.ts +++ b/packages/cli/src/utils/validate-expressions.ts @@ -10,8 +10,9 @@ * agent `validate_expression` tool exactly. * * Scope (v1): flow predicates (start/decision `config.condition` + edge - * `condition`) and object validation-rule / formula predicates. Each error is - * located (flow/object + node/edge/field) with a corrective message. + * `condition`), object validation-rule / formula predicates, and UI action + * `visible` / `disabled` predicates. Each error is located (flow/object/action + * + node/edge/field) with a corrective message. */ import { validateExpression } from '@objectstack/formula'; @@ -160,5 +161,37 @@ export function validateStackExpressions(stack: AnyRec): ExprIssue[] { } } + // ── Action `visible` / `disabled` predicates ─────────────────────── + // Record-scoped, same as validation rules: a record-header / row action's + // `visible` is evaluated by ActionEngine against `{ record, recordId, + // objectName, user, … }` with fail-closed semantics, so a BARE field ref + // (`done` instead of `record.done`) throws and the action is silently hidden + // on every record (the trap behind the #2183 "Mark Done never hides" hunt). + // Flagging it here turns that into a build error with a corrective message. + // `disabled` may be a boolean (skip) or a predicate (check). + const seenActions = new Set(); + const checkAction = (where: string, action: AnyRec, objectName?: string): void => { + const obj = objectName + ?? (typeof action.objectName === 'string' ? action.objectName : undefined) + ?? (typeof action.object === 'string' ? action.object : undefined); + const name = typeof action.name === 'string' ? action.name : '?'; + const key = `${obj ?? ''}:${name}`; + if (seenActions.has(key)) return; // de-dup (actions are merged onto objects AND kept top-level) + seenActions.add(key); + check(`${where} · action '${name}' visible`, action.visible, obj, 'record'); + if (typeof action.disabled !== 'boolean') { + check(`${where} · action '${name}' disabled`, action.disabled, obj, 'record'); + } + }; + for (const action of asArray(stack.actions)) { + checkAction('stack', action); + } + for (const obj of objects) { + const objectName = typeof obj.name === 'string' ? obj.name : undefined; + for (const action of asArray(obj.actions)) { + checkAction(`object '${objectName}'`, action, objectName); + } + } + return issues; } diff --git a/packages/formula/src/cel-engine.ts b/packages/formula/src/cel-engine.ts index 2102a7ca21..99edce72aa 100644 --- a/packages/formula/src/cel-engine.ts +++ b/packages/formula/src/cel-engine.ts @@ -53,6 +53,9 @@ const SCOPE_ROOTS = [ 'record', 'previous', 'input', 'output', 'os', 'vars', 'variables', 'automation', 'context', 'args', 'item', 'env', 'user', 'step', 'result', 'trigger', 'event', 'payload', 'data', 'params', 'config', 'settings', + // UI action / predicate context (ActionEngine, renderers): the current + // record plus ambient globals exposed to `visible`/`disabled` predicates. + 'ctx', 'features', ] as const; /**