From 6eada82555bc2d274e43c3b26d7a21b5de7c2a05 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 07:37:07 +0000 Subject: [PATCH 1/2] fix(core): accept declared predicates on the visible / disabled schema rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `BASE_SCHEMA_RULES` validated both keys with `typeof value === 'boolean'`, so every expression-valued gate — the authoring form AGENTS.md section 4 declares, `SchemaRenderer` evaluates and `@objectstack/spec` normalizes into a `{ dialect, source }` envelope — was reported ` must be a boolean` by the dev-mode validator, and its host element got `data-obj-schema-invalid`. The verdict now delegates to `hasDeclaredPredicate`, the repo's single definition of "is a predicate gate declared on this value?", rather than being answered a second time here. The two keys stay in the table and the rule still bites: a number, `null`, `{}`, an array, `''`, whitespace-only predicate text and the empty / blank-`source` envelope are all still reported. The accept set is a strict superset of the one it replaces. Card: objectui#6505 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- .../6505-predicate-valued-gate-rules.md | 42 ++++++ .../predicate-valued-gate-rules.test.ts | 133 ++++++++++++++++++ .../core/src/validation/schema-validator.ts | 78 ++++++++-- 3 files changed, 243 insertions(+), 10 deletions(-) create mode 100644 .changeset/6505-predicate-valued-gate-rules.md create mode 100644 packages/core/src/validation/__tests__/predicate-valued-gate-rules.test.ts diff --git a/.changeset/6505-predicate-valued-gate-rules.md b/.changeset/6505-predicate-valued-gate-rules.md new file mode 100644 index 0000000000..67f1d4c700 --- /dev/null +++ b/.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. diff --git a/packages/core/src/validation/__tests__/predicate-valued-gate-rules.test.ts b/packages/core/src/validation/__tests__/predicate-valued-gate-rules.test.ts new file mode 100644 index 0000000000..a488cb2dd9 --- /dev/null +++ b/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.`. */ +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 = [ + ['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 = [ + ['`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 = [ + ['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 }); + } + }); +}); diff --git a/packages/core/src/validation/schema-validator.ts b/packages/core/src/validation/schema-validator.ts index 6f09ac68ad..e27f547b7d 100644 --- a/packages/core/src/validation/schema-validator.ts +++ b/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 `" 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') }; /** From 36ba920807d9ca640c4f8ab7dda16425af168bc9 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 07:57:53 +0000 Subject: [PATCH 2/2] test(react): lower the one raw-total dev pin that counted the removed false positive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `SchemaRenderer.disabledGateFaultDiagnostic.test.tsx`'s group-1 development cell pinned the RAW warn total at 2: its own fault line, plus core's `disabled must be a boolean` line for the same node. That second line was the false positive this branch removes, so the dev build now prints exactly one. Measured on the merged head before changing anything: across `packages/react/` and `packages/core/` (162 files, 2951 tests) exactly one cell moved, and it is this one. The count is lowered to the measured reality, not loosened — it still pins that nothing else reaches the console. The helper docblock that explained the old 2 is corrected in the same hunk: it asserted the false positive as present-tense current behaviour, which this branch makes untrue. The by-name subtraction it justifies is unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q --- ...derer.disabledGateFaultDiagnostic.test.tsx | 43 ++++++++++++------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/packages/react/src/__tests__/SchemaRenderer.disabledGateFaultDiagnostic.test.tsx b/packages/react/src/__tests__/SchemaRenderer.disabledGateFaultDiagnostic.test.tsx index 37f67850ed..079a6f947b 100644 --- a/packages/react/src/__tests__/SchemaRenderer.disabledGateFaultDiagnostic.test.tsx +++ b/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); }); }); });