fix(core): accept declared predicates on the visible / disabled schema rules

.changeset/6505-predicate-valued-gate-rules.md

@@ -0,0 +1,42 @@
+---
+'@object-ui/core': patch
+---
+
+Dev-mode `validateSchema` no longer reports every expression-valued `visible` / `disabled`
+gate as an invalid schema (objectui#6505). `BASE_SCHEMA_RULES` declared both keys
+`typeof value === 'boolean'`, so the exact authoring form the docs teach —
+`{ "type": "button", "disabled": "${record.stage == 'closed'}" }` — printed
+`disabled must be a boolean` and its host element got `data-obj-schema-invalid`, the cue
+apps are told to hang a red outline off.
+
+**The accept set widens to what the protocol already declares and the runtime already
+accepts, not beyond it.** `AGENTS.md` §4 declares both keys as expressions, `SchemaRenderer`
+evaluates them through `hasDeclaredPredicate` + `evaluateCondition`, `@objectstack/spec`
+normalizes every authored predicate into a `{ dialect, source }` envelope, and the
+objectui#3862 / objectui#3955 rulings are entirely about which expression spellings count
+as declared. This table was the one place in the repo that disagreed, so this restores
+declared = enforced rather than changing a contract.
+
+**The rule still bites, and that half is pinned separately.** The two keys stay in
+`BASE_SCHEMA_RULES`: a number, `null`, `{}`, an array, `''`, whitespace-only predicate text
+and the empty / blank-`source` envelope (objectui#3960) are all still reported at their own
+path with `INVALID_TYPE`. Every one of those was reported before this change too — the
+accept set is a strict superset of the old one, so nothing that validated stops validating
+and nothing refused becomes accepted. The message now names both halves of the accept set
+instead of only the half that did not change.
+
+The verdict is delegated to `hasDeclaredPredicate` (`evaluator/declaredPredicate.ts`), the
+repo's single definition of "is a predicate gate declared on this value?"
+(objectui#3850's ruling), rather than answered a second time in the validator — a
+hand-rolled twin that agrees today and drifts tomorrow is the defect class this rule was
+already an instance of. `packages/core/src/validation/__tests__/predicate-valued-gate-rules.test.ts`
+pins the delegation behaviourally: the rule's verdict must equal
+`boolean || hasDeclaredPredicate(value)` across every probe in the file.
+
+The explicit boolean arm is kept even though `hasDeclaredPredicate` already subsumes it, so
+the superset relationship is provable locally: a future narrowing on the declaredness side
+cannot silently start reporting `disabled: false` — the most explicit gate an author can
+write — as an invalid schema.
+
+The zod `safeValidateSchema` surface (`@object-ui/types/zod`, objectui#6318) is a different
+validator and is untouched.

packages/core/src/validation/__tests__/predicate-valued-gate-rules.test.ts

@@ -0,0 +1,133 @@
+/**
+ * ObjectUI
+ * Copyright (c) 2024-present ObjectStack Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * objectui#6505 — `BASE_SCHEMA_RULES` used to declare `visible` / `disabled`
+ * "must be a boolean", so every EXPRESSION-valued gate — the authoring form
+ * `AGENTS.md` §4 documents and `SchemaRenderer` evaluates — was reported as an
+ * invalid schema by the dev-mode validator.
+ *
+ * The two directions this file exists to hold apart, because a fix that only
+ * proves the first is indistinguishable from having DELETED the rule:
+ *
+ * 1. the false positive is gone — a declared predicate validates;
+ * 2. the rule STILL BITES — a value that is neither a boolean nor a declared
+ * predicate is still reported, at its own path, with `INVALID_TYPE`.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { validateSchema } from '../schema-validator';
+import { hasDeclaredPredicate } from '../../evaluator/declaredPredicate';
+
+/** The two keys this card is about, asserted identically — they are one rule. */
+const GATE_KEYS = ['visible', 'disabled'] as const;
+
+/** Every error message the validator produced for `schema.<key>`. */
+function gateMessages(key: string, value: unknown): string[] {
+ const result = validateSchema({ type: 'button', [key]: value });
+ return result.errors.filter((e) => e.path === `schema.${key}`).map((e) => e.message);
+}
+
+/**
+ * Predicate spellings the runtime ALREADY accepts — `toPredicateInput` folds
+ * none of them to `undefined`, so `evaluateCondition` reaches a verdict on each.
+ * Named individually rather than as "any string" so a narrowing of the accept
+ * set cannot pass by keeping one spelling alive.
+ */
+const DECLARED_PREDICATES: ReadonlyArray<readonly [string, unknown]> = [
+ ['a bare CEL expression (the docs shorthand)', "record.stage == 'closed'"],
+ ['a `${…}` template string (AGENTS.md section 4 spelling)', "${record.stage == 'closed'}"],
+ ['a `cel` envelope (what `@objectstack/spec` normalizes to)', { dialect: 'cel', source: "record.stage == 'closed'" }],
+ ['a non-`cel` envelope (flattened onto the legacy path)', { dialect: 'template', source: '${record.locked}' }],
+];
+
+/**
+ * The OLD accept set. Kept as its own list, asserted separately, so this file
+ * pins that the fix WIDENS rather than moves: every value that validated before
+ * still validates.
+ */
+const BOOLEANS: ReadonlyArray<readonly [string, unknown]> = [
+ ['`true`', true],
+ ['`false` — a verdict, not an absent gate (objectui#3812)', false],
+];
+
+/**
+ * Values that are neither a boolean nor a declared predicate. `hasDeclaredPredicate`
+ * answers `false` for each — the junk arm of objectui#3850's ruling — and the
+ * rule must keep reporting them. Every entry here was reported BEFORE this fix
+ * too: the accept set widens, and nothing that was refused becomes accepted.
+ */
+const NOT_A_GATE: ReadonlyArray<readonly [string, unknown]> = [
+ ['a number', 0],
+ ['a truthy number', 1],
+ ['`null` — no predicate at all (objectui#3862)', null],
+ ['an empty object', {}],
+ ['an array', []],
+ ['an empty string (objectui#3492 / objectui#3842)', ''],
+ ['whitespace-only predicate text (objectui#3960)', ' '],
+ ['an empty `cel` envelope (objectui#3850)', { dialect: 'cel', source: '' }],
+ ['a blank-`source` envelope (objectui#3960)', { dialect: 'cel', source: ' ' }],
+];
+
+describe.each(GATE_KEYS)('#6505 — `%s` accepts every DECLARED predicate spelling', (key) => {
+ it.each(DECLARED_PREDICATES)('accepts %s', (_label, value) => {
+ expect(gateMessages(key, value)).toEqual([]);
+ });
+
+ it.each(BOOLEANS)('still accepts %s — the old accept set is a SUBSET of the new one', (_label, value) => {
+ expect(gateMessages(key, value)).toEqual([]);
+ });
+
+ it('validates the node this card is named after, end to end', () => {
+ const result = validateSchema({ type: 'button', [key]: "${record.stage == 'closed'}" });
+ expect(result.valid).toBe(true);
+ // The message this card is named after, asserted by NAME rather than by
+ // count: `valid: true` alone would also go green on a deleted rule.
+ expect(result.errors.map((e) => e.message)).not.toContain(`${key} must be a boolean`);
+ });
+});
+
+describe.each(GATE_KEYS)('#6505 — the `%s` rule STILL BITES', (key) => {
+ it.each(NOT_A_GATE)('reports %s', (_label, value) => {
+ const result = validateSchema({ type: 'button', [key]: value });
+ expect(result.valid).toBe(false);
+ const errors = result.errors.filter((e) => e.path === `schema.${key}`);
+ expect(errors).toHaveLength(1);
+ expect(errors[0].code).toBe('INVALID_TYPE');
+ // The message must name BOTH halves of the accept set, or an author reading
+ // it learns only the half that did not change.
+ expect(errors[0].message).toContain(key);
+ expect(errors[0].message).toContain('boolean');
+ expect(errors[0].message).toContain('expression');
+ });
+
+ it('the rule is still IN the table — a deleted rule reports nothing at all', () => {
+ // The forbidden option, pinned as a behaviour: dropping `visible` /
+ // `disabled` from `BASE_SCHEMA_RULES` makes this whole describe block go
+ // green by reporting NOTHING, so one cell asserts the rule fires at all.
+ expect(validateSchema({ type: 'button', [key]: 0 }).errors).not.toHaveLength(0);
+ });
+});
+
+/**
+ * The drift pin. The repo owns ONE definition of "is this a declared
+ * predicate?" (objectui#3850) and this validator must not grow a second
+ * answer — a hand-rolled twin agreeing today and drifting tomorrow is the
+ * defect class this card belongs to.
+ */
+describe('#6505 — the validator delegates to `hasDeclaredPredicate`, it does not re-answer', () => {
+ const ALL = [...DECLARED_PREDICATES, ...BOOLEANS, ...NOT_A_GATE];
+
+ it.each(GATE_KEYS)('the `%s` verdict equals `boolean || hasDeclaredPredicate` for every probe', (key) => {
+ for (const [label, value] of ALL) {
+ const accepted = gateMessages(key, value).length === 0;
+ const expected = typeof value === 'boolean' || hasDeclaredPredicate(value);
+ expect({ label, accepted }).toEqual({ label, accepted: expected });
+ }
+ });
+});

packages/core/src/validation/schema-validator.ts

@@ -17,6 +17,7 @@
  */
 
 import type { BaseSchema } from '@object-ui/types';
+import { hasDeclaredPredicate } from '../evaluator/declaredPredicate.js';
 
 /**
  * One issue found while walking an ObjectUI schema TREE — `path` locates the
@@ -53,6 +54,69 @@ export interface SchemaNodeValidationResult {
  warnings: SchemaNodeValidationError[];
 }
 
+/**
+ * The rule for a PREDICATE GATE key (`visible` / `disabled`) — objectui#6505.
+ *
+ * ## What this rule used to say, and what it cost
+ *
+ * Both keys read `typeof value === 'boolean'`, message `"<key> must be a
+ * boolean"`. Both keys are EXPRESSIONS everywhere else in the system: AGENTS.md
+ * §4 declares the protocol as `hidden?: string; // expression` /
+ * `disabled?: string; // expression`, `SchemaRenderer` evaluates them through
+ * `hasDeclaredPredicate` + `evaluateCondition`, `@objectstack/spec` normalizes
+ * every authored predicate into a `{ dialect, source }` envelope, and the
+ * objectui#3862 / objectui#3955 rulings are entirely about which EXPRESSION
+ * spellings count as declared. So the node the docs teach —
+ * `{ type: 'button', disabled: "${record.stage == 'closed'}" }` — was reported
+ * `disabled must be a boolean` by the dev-mode validator and its host element
+ * got `data-obj-schema-invalid`, the cue apps hang a red outline off.
+ *
+ * `BASE_SCHEMA_RULES` was the ONE place in the repo that disagreed with the
+ * protocol, so this restores declared = enforced rather than widening a
+ * contract: the accept set moves to exactly what the runtime already accepts.
+ *
+ * ## Why `hasDeclaredPredicate` and not a check written here
+ *
+ * `evaluator/declaredPredicate.ts` is the repo's single definition of "is a
+ * predicate gate DECLARED on this value?" (objectui#3850's ruling; nothing in
+ * the repo asks that question anywhere else). A second, hand-rolled answer in
+ * this table is how the validator and the renderer come to disagree about the
+ * same value — which is the defect class this rule was already an instance of.
+ * The delegation is behavioural, so it is pinned behaviourally: the drift pin in
+ * `__tests__/predicate-valued-gate-rules.test.ts` asserts this rule's verdict
+ * equals `boolean || hasDeclaredPredicate(value)` across every probe.
+ *
+ * ## Why the boolean arm survives even though it is subsumed
+ *
+ * `hasDeclaredPredicate(true)` and `hasDeclaredPredicate(false)` are both
+ * `true` today (`toPredicateInput` returns a boolean unchanged), so the first
+ * arm changes no verdict. It is kept because it makes this rule's accept set a
+ * provable SUPERSET of the one it replaces, locally: a future narrowing on the
+ * declaredness side cannot silently start reporting `disabled: false` — the
+ * most explicit gate an author can write — as an invalid schema.
+ *
+ * ## What it still REFUSES (the half that is not negotiable)
+ *
+ * Everything `hasDeclaredPredicate` calls junk: a number, `null`, `{}`, an
+ * array, `''`, whitespace-only predicate text, and the empty / blank-`source`
+ * envelope (objectui#3960). Every one of those was refused before this change
+ * too — the accept set widens and nothing refused becomes accepted. Dropping
+ * the two keys from this table was the option this fix was explicitly forbidden
+ * to take: an absent rule reports nothing at all, which is gate weakening
+ * wearing the same green.
+ */
+function predicateGateRule(key: 'visible' | 'disabled') {
+ return {
+ required: false,
+ validate: (value: unknown): boolean =>
+ typeof value === 'boolean' || hasDeclaredPredicate(value),
+ message:
+ `${key} must be a boolean or a declared predicate: an expression string ` +
+ `(bare, e.g. "record.stage == 'closed'", or the "\${...}" template ` +
+ `spelling), or a { dialect, source } expression envelope`,
+ };
+}
+
 /**
  * Validation rules for base schema
  */
@@ -72,16 +136,10 @@ const BASE_SCHEMA_RULES = {
  validate: (value: any) => typeof value === 'string',
  message: 'className must be a string'
  },
- visible: {
- required: false,
- validate: (value: any) => typeof value === 'boolean',
- message: 'visible must be a boolean'
- },
- disabled: {
- required: false,
- validate: (value: any) => typeof value === 'boolean',
- message: 'disabled must be a boolean'
- }
+ // objectui#6505 — both keys are EXPRESSIONS in the protocol, not booleans.
+ // See `predicateGateRule` above for the accept set and what it still refuses.
+ visible: predicateGateRule('visible'),
+ disabled: predicateGateRule('disabled')
 };
 
 /**

packages/react/src/__tests__/SchemaRenderer.disabledGateFaultDiagnostic.test.tsx

@@ -141,19 +141,24 @@ const visibilityReports = (warn: WarnSpy): string[] =>
 const allWarnings = (warn: WarnSpy): string[] => warn.mock.calls.map((c) => String(c[0]));
 
 /**
- * The ONE pre-existing line a development build prints beside these
- * diagnostics, and it is not ours.
+ * The line a development build could print beside these diagnostics without it
+ * being ours — `SchemaRenderer` runs `validateSchemaOnce` in dev only.
  *
- * `SchemaRenderer` runs `validateSchemaOnce` in dev only, and core's
- * `BASE_SCHEMA_RULES` declares `disabled` "must be a boolean" — so every
- * EXPRESSION-valued `disabled`, the authoring form this whole card is about,
- * is also reported as an invalid schema. That is a false positive predating
- * this card and filed separately (objectui#6505); it is named here rather than
- * absorbed into a loose assertion, because "the total was 2" would go equally
- * green on a build that printed our line twice.
+ * HISTORY, because the numbers below moved with it (objectui#6505): when this
+ * file was written, core's `BASE_SCHEMA_RULES` declared `disabled` "must be a
+ * boolean", so every EXPRESSION-valued `disabled` — the authoring form this
+ * whole card is about — was ALSO reported as an invalid schema. That false
+ * positive is gone: the rule now accepts a declared predicate, and the dev
+ * build prints nothing here. Production was never affected — that validator is
+ * a no-op there — which is why the production cases below always pinned the
+ * RAW total.
  *
- * Production is unaffected — that validator is a no-op there — which is why
- * the production cases below can still pin the RAW total.
+ * The by-name subtraction stays, and is not now-redundant belt-and-braces: it
+ * is what makes every `nonValidatorWarnings` assertion in this file invariant
+ * under whatever core's validator decides to say. That property is why this
+ * file survived objectui#6505 with ONE number changed instead of a re-audit —
+ * "the total was 2" would have gone equally green on a build that printed our
+ * line twice, and would have said nothing about which line was which.
  */
 const DEV_SCHEMA_VALIDATOR_NOISE = '[ObjectUI] Invalid schema detected:';
 /** Everything printed that is NOT the known dev-only validator line. */
@@ -364,12 +369,18 @@ describe('#6445 group 1 — a faulting `disabled` gate is loud, and still disabl
  expect(disabledProp()).toBe('true');
  expect(reports(warn)).toHaveLength(1);
  expect(reports(warn)[0]).toContain(FAULT_BARE_DEV);
- // ONE fault, ONE line of ours. The dev build also prints core's
- // `disabled must be a boolean` validator line for this very node
- // (objectui#6505) — subtracted by NAME rather than by count, so a second
- // copy of OUR line could never hide inside the allowance.
+ // ONE fault, ONE line of ours — subtracted by NAME rather than by count,
+ // so a second copy of OUR line could never hide inside the allowance.
  expect(nonValidatorWarnings(warn)).toHaveLength(1);
- expect(allWarnings(warn)).toHaveLength(2);
+ // The RAW total was 2 when this cell was written: ours, plus core's
+ // `disabled must be a boolean` line for this very node. That second line
+ // was a FALSE POSITIVE — an expression-valued `disabled` is the authoring
+ // form the protocol declares — and objectui#6505 removed it, so the dev
+ // build now prints exactly one line here. The count is lowered to the
+ // measured reality and NOT loosened: this still pins that nothing else
+ // reaches the console, which is the whole reason the raw total is worth
+ // asserting beside the by-name one above.
+ expect(allWarnings(warn)).toHaveLength(1);
  });
  });
 });