diff --git a/packages/spec/scripts/check-react-blocks-declaration-parity.test.ts b/packages/spec/scripts/check-react-blocks-declaration-parity.test.ts index 52820a051c..2b92631643 100644 --- a/packages/spec/scripts/check-react-blocks-declaration-parity.test.ts +++ b/packages/spec/scripts/check-react-blocks-declaration-parity.test.ts @@ -371,3 +371,123 @@ describe('check:react-declaration-parity — SDUI object-* blocks (#7751)', () = expect(status, output).toBe(0); }); }); + +/** + * THE ACCEPTED SET IS THE FULL NODE CONTRACT — AND STAYS CALIBRATED (#13192). + * + * The gate used to judge a registry input against `ComponentPropsMap[type]` + * alone: the PER-BLOCK half of what a page node may carry. The other half is + * declared once on `PageComponentSchema` and applies to every component, so a + * NODE-LEVEL key read to this gate as an invented input and the complaint was + * FALSE — not merely noisy. Measured on the ref that fixed it: + * + * PageComponentSchema.safeParse({ type: 'list-view', dataSource: {…} }) + * → success: true, output KEEPS `dataSource` + * ComponentPropsMap['object-grid'] → 37 keys, none of them `dataSource` + * + * objectui PR #6767 landed 17 renderers across 12 packages that declare + * `dataSource` at the `ElementDataSourceGate` wrapping seam, so this was the + * INVERSE of the usual gate defect: not "should be red and isn't" but + * "shouldn't be red and is about to be", which blocks a correct change with a + * false reason. + * + * ⭐ THE HALF THAT MATTERS MOST HERE IS THE NEGATIVE ONE. Widening an accepted + * set is one edit away from making a gate vacuous, and a vacuous gate is worse + * than the false failure it replaced because nothing ever says so again. These + * pins therefore assert BOTH directions in the same shape of run: `dataSource` + * and `className` accepted, while `viewName`, a retired node-level key, and an + * invented name still go red and NAME themselves. + */ +describe('check:react-declaration-parity — the node contract, and its calibration (#13192)', () => { + const CLEAN: BaselineFile = { blocks: { 'object-grid': { registryOnly: [], missing: false } } }; + /** A real `object-grid` prop, so every fixture below also exercises the props half. */ + const BLOCK_PROP = 'objectName'; + + /** + * The derived set itself, read off the report. + * + * It is printed on every run for the same reason the scope note is (#4472): a + * widening nobody can see in the CI log cannot be audited. Pinning the printed + * line is what makes "derived, not listed" checkable — the set must come from + * `PageComponentSchema`'s key shape, and a future node-level key must appear + * here the day it lands without anyone editing this file. + */ + it('prints the node-level accepted set it derived, and derives it from the schema shape', { timeout: SPAWN_TIMEOUT_MS }, () => { + const out = run(manifestFor('object-grid', [BLOCK_PROP])); + expect(out).toMatch(/DERIVED from PageComponentSchema's own key shape/); + const line = out.split('\n').find((l) => l.includes('aria, className')) ?? ''; + const derived = line.trim().split(/,\s*/); + // ⭐ ACCEPTED — the two keys the card names. + expect(derived).toContain('dataSource'); + expect(derived).toContain('className'); + // ⭐ REFUSED — `objectName` and `viewName` are NOT node-level. `objectName` gets + // no behavioural leg below because it is accepted on every block for a + // legitimate PER-BLOCK reason (measured: it is a prop of all six `object-*` + // schemas and an overlay prop of all three react blocks), so the discrimination + // that can actually go wrong — it being swept in as node-level — is exactly this. + expect(derived).not.toContain('objectName'); + expect(derived).not.toContain('viewName'); + // ⭐ REFUSED — a `retiredKey()` tombstone is IN the shape but accepts nothing. + expect(derived).not.toContain('responsive'); + }); + + it('accepts a node-level key the block\'s own props schema does not declare (dataSource)', { timeout: SPAWN_TIMEOUT_MS }, () => { + const { status, output } = runExit({ + manifest: manifestFor('object-grid', [BLOCK_PROP, 'dataSource']), + baseline: CLEAN, + args: ['--strict'], + }); + // Accepted, and SAID SO rather than silently dropped. + expect(output).toMatch(/spec accepts at NODE level \(not a per-block prop\): dataSource/); + expect(output).not.toMatch(/registry declares, spec does not: .*dataSource/); + expect(output).toMatch(/no new DECLARATION divergence/); + expect(status, output).toBe(0); + }); + + it('accepts the other node-level key the card checked (className)', { timeout: SPAWN_TIMEOUT_MS }, () => { + const { status, output } = runExit({ + manifest: manifestFor('object-grid', [BLOCK_PROP, 'className']), + baseline: CLEAN, + args: ['--strict'], + }); + expect(output).toMatch(/spec accepts at NODE level \(not a per-block prop\): className/); + expect(status, output).toBe(0); + }); + + /** + * The calibration proper: a key that is neither a node-level key nor a prop of + * this block must still go red — IN THE SAME RUN in which a node-level key is + * accepted, so the two cannot be confused for one lenient mode. + */ + it.each([ + ['viewName', 'a plausible-looking key that is on no half of the contract'], + ['responsive', 'a node-level key the shape RETIRES — the tombstone must not be swept in'], + ['zzzInventedRegistryInput', 'an outright invented input'], + ])('still refuses %s (%s) while accepting dataSource in the same run', (key, _why) => { + const { status, output } = runExit({ + manifest: manifestFor('object-grid', [BLOCK_PROP, 'dataSource', key]), + baseline: CLEAN, + args: ['--strict'], + }); + expect(output).toContain(`new registry-only input(s) not in baseline: ${key}`); + // The accept and the refusal are simultaneous — this is the discrimination. + expect(output).toMatch(/spec accepts at NODE level \(not a per-block prop\): dataSource/); + expect(status, output).toBe(1); + }, SPAWN_TIMEOUT_MS); + + /** + * The react-block half of the gate takes the same fix. `list-view`'s schema + * declares 49 props and `dataSource` is not among them, so before #13192 a + * `` declaring it was reported as a new registry-only input. + */ + it('applies to the react-block half too, not only the SDUI object-* half', { timeout: SPAWN_TIMEOUT_MS }, () => { + const { status, output } = runExit({ + manifest: manifestFor('list-view', ['dataSource']), + baseline: { blocks: { ListView: { registryOnly: [], missing: false } } }, + args: ['--strict'], + }); + expect(output).toMatch(/ \(list-view\):.*node-level/); + expect(output).toMatch(/spec accepts at NODE level \(not a per-block prop\): dataSource/); + expect(status, output).toBe(0); + }); +}); diff --git a/packages/spec/scripts/check-react-blocks-declaration-parity.ts b/packages/spec/scripts/check-react-blocks-declaration-parity.ts index c9d8cc09ed..850d5a6ec0 100644 --- a/packages/spec/scripts/check-react-blocks-declaration-parity.ts +++ b/packages/spec/scripts/check-react-blocks-declaration-parity.ts @@ -50,6 +50,23 @@ // fixture that stays green while its "renderer" ignores everything, so the // capability cannot be re-assumed by the next reader. // +// THE SPEC SIDE IS THE WHOLE NODE CONTRACT, NOT ONLY THE PROPS HALF (#13192). +// +// `ComponentPropsMap[type]` is the PER-BLOCK half of what a page node may carry. +// The other half is declared ONCE, on `PageComponentSchema` itself — `type`, +// `id`, `className`, `dataSource`, `visibleWhen`, … — and applies to every +// component whatever its block. Judging registry inputs against the props half +// alone makes a node-level key read as an INVENTED input, and that complaint is +// FALSE rather than merely inconvenient: the spec does accept the key. +// +// The two readings that establish it, re-measured on this ref before the fix: +// +// PageComponentSchema.safeParse({ type: 'list-view', dataSource: {…} }) +// → success: true, and the parsed output KEEPS `dataSource` +// ComponentPropsMap['object-grid'] → 37 keys, `dataSource` among none of them +// +// So the accepted set per block is node-level keys ∪ per-block props ∪ overlay. +// // WHERE THE MANIFEST COMES FROM — AND WHY NOTHING HERE CAN PRODUCE ONE (#4690). // // The right-hand side is objectui's registry-inputs manifest (sdui.manifest.json). @@ -99,6 +116,7 @@ import fs from 'fs'; import { z } from 'zod'; import { REACT_BLOCKS } from '../src/ui/react-blocks'; import { ComponentPropsMap } from '../src/ui/component.zod'; +import { PageComponentSchema } from '../src/ui/page.zod'; /** * The SDUI `object-*` page blocks whose props schemas entered @@ -119,6 +137,89 @@ const SDUI_OBJECT_BLOCK_TYPES = Object.keys(ComponentPropsMap) .filter((t) => t.startsWith('object-')) .sort(); +/** + * Exit loudly because the NODE-LEVEL half of the accepted set could not be read. + * + * Same posture as {@link cannotRun} and for the same reason (#4690): a derivation + * that silently comes back empty does not make this gate lenient, it makes it + * catastrophically strict — every node-level key on every block would report as an + * invented registry input, i.e. the exact false failure #13192 exists to remove, + * restored by a shrug. Independent of `--strict`: no comparison can be trusted. + */ +function cannotDeriveNodeContract(reason: string): never { + console.error(`✗ react-blocks declaration parity: ${reason}`); + console.error(' This gate did NOT run. That is a failure, not a skip (#4690/#13192).'); + console.error( + [ + '', + " The accepted set per block is the FULL node contract — PageComponentSchema's own", + " node-level keys UNION the block's ComponentPropsMap props — and the node-level half", + ' is derived from the shape, never listed (#13192).', + '', + ' PageComponentSchema ends in `.transform(normalizeVisibleWhen)`, so it is a ZodPipe and', + ' `z.toJSONSchema` of the pipe itself yields NO properties; the authorable key shape is', + ' the pipe INPUT side, `_def.in`. If zod moved that internal, re-point the reader in', + ' `deriveNodeContractKeys()` below.', + '', + ' ⛔ Do NOT "fix" this by restoring a hand-listed key set. The list this derivation', + ' replaced named five of the twelve keys the shape accepts, and each of the other', + ' seven was one registry declaration away from the same false failure.', + ].join('\n'), + ); + process.exit(1); +} + +/** + * A JSON-Schema entry that accepts NOTHING — `{ not: {} }`, which is how + * `z.toJSONSchema` spells `z.never()`. + * + * That is what a key retired with `retiredKey()` looks like: the tombstone stays + * in the shape (so `z.input` keeps typing it `never` and the parse keeps the + * removal's prescription) while the spec refuses every value. `responsive` is one + * today. ⚠️ A tombstone must NOT join the accepted set — the spec does not accept + * the key, so a registry still declaring it as an input is a REAL finding and has + * to stay red. Sweeping the whole shape in without this discrimination is how a + * widening turns a gate vacuous. + */ +function acceptsNothing(entry: unknown): boolean { + if (!entry || typeof entry !== 'object') return false; + const not = (entry as { not?: unknown }).not; + return !!not && typeof not === 'object' && Object.keys(not as object).length === 0; +} + +/** + * The node-level half of the accepted set, read off `PageComponentSchema`'s own + * key shape — DERIVED, never listed, so the node-level key that lands next month + * is covered the day it lands rather than after it has produced a false failure. + * + * Measured on this ref: twelve keys — `aria`, `className`, `dataSource`, `events`, + * `id`, `label`, `properties`, `responsiveStyles`, `style`, `type`, `visibility`, + * `visibleWhen` — plus the `responsive` tombstone, which {@link acceptsNothing} + * drops. The literal this replaced listed five of those twelve. + */ +function deriveNodeContractKeys(): Set { + const pipe = PageComponentSchema as unknown as { _def?: { in?: unknown } }; + const shapeSource = pipe?._def?.in ?? PageComponentSchema; + let js: any; + try { + js = z.toJSONSchema(shapeSource as any, { unrepresentable: 'any' } as any); + } catch (err) { + cannotDeriveNodeContract( + `PageComponentSchema's key shape is unreadable — z.toJSONSchema threw: ${(err as Error).message}`, + ); + } + if (js?.$ref && js?.$defs) js = js.$defs[String(js.$ref).split('/').pop()!] ?? js; + const props = js?.properties; + if (!props || typeof props !== 'object' || Object.keys(props).length === 0) { + cannotDeriveNodeContract( + "PageComponentSchema's key shape read as EMPTY — no node-level key could be derived.", + ); + } + return new Set(Object.keys(props).filter((k) => !acceptsNothing(props[k]))); +} + +const NODE_CONTRACT_KEYS = deriveNodeContractKeys(); + const MANIFEST = process.env.MANIFEST; const FAIL_ON_DIVERGENCE = process.argv.includes('--strict'); const UPDATE_BASELINE = process.argv.includes('--update'); @@ -143,11 +244,39 @@ const SCOPE_NOTE = ' No renderer is inspected. A prop BOTH sides declare and NO renderer reads counts\n' + ' as agreement here; that blind spot is what #4413 shipped through (see #4472).'; +/** How an accepted node-level input is named in the per-block detail lines. */ +const NODE_LEVEL_ACCEPT_LABEL = 'registry declares, spec accepts at NODE level (not a per-block prop)'; + +/** + * What the gate accepted at node level, printed on EVERY run. + * + * Same reason the scope note is in the output rather than only in this header + * (#4472): a widening that is invisible in the CI log cannot be audited, and the + * whole risk of widening an accepted set is that it quietly becomes everything. + * Naming the twelve keys — and saying that tombstones are excluded — makes the + * calibration readable from the log a person actually forms a belief from. + */ +const NODE_CONTRACT_NOTE = + `Node contract: the accepted set per block is node-level keys ∪ this block's props ∪ overlay.\n` + + ` ${NODE_CONTRACT_KEYS.size} node-level keys, DERIVED from PageComponentSchema's own key shape (#13192):\n` + + ` ${[...NODE_CONTRACT_KEYS].sort().join(', ')}\n` + + ' Keys the shape RETIRES (`z.never()` tombstones) are excluded, so a registry input\n' + + ' naming one is still reported — a widening that swept those in would be vacuous.'; + +/** + * The PER-BLOCK half of the accepted set: the props this block's own schema + * declares, minus anything the node-level contract already carries. + * + * The subtraction used to be a five-name literal (`aria`, `type`, `id`, + * `className`, `style`); it is now {@link NODE_CONTRACT_KEYS}, the same derived + * set the registry side is judged against — one source for "what is node-level", + * because two would be the same drift kept in two places (#13192). + */ function specProps(schema: any): string[] { try { let js: any = z.toJSONSchema(schema, { unrepresentable: 'any' } as any); if (js?.$ref && js?.$defs) js = js.$defs[String(js.$ref).split('/').pop()!] ?? js; - return Object.keys(js?.properties ?? {}).filter((k) => !['aria', 'type', 'id', 'className', 'style'].includes(k)); + return Object.keys(js?.properties ?? {}).filter((k) => !NODE_CONTRACT_KEYS.has(k)); } catch { return []; } @@ -240,6 +369,7 @@ const current: Record = {}; console.log('# Spec ↔ registry declaration parity (react blocks)\n'); console.log(SCOPE_NOTE + '\n'); +console.log(NODE_CONTRACT_NOTE + '\n'); for (const b of REACT_BLOCKS) { if (!b.schema) continue; const spec = new Set(specProps(b.schema)); @@ -253,15 +383,17 @@ for (const b of REACT_BLOCKS) { const inputSet = new Set(inputs); const ov = overlay(b); const specOnly = [...spec].filter((p) => !inputSet.has(p) && !ov.has(p)); - const registryOnly = [...inputSet].filter((p) => !spec.has(p) && !ov.has(p)); + const nodeLevel = [...inputSet].filter((p) => NODE_CONTRACT_KEYS.has(p) && !spec.has(p) && !ov.has(p)); + const registryOnly = [...inputSet].filter((p) => !spec.has(p) && !ov.has(p) && !NODE_CONTRACT_KEYS.has(p)); const declaredByBoth = [...spec].filter((p) => inputSet.has(p)); totalSpecOnly += specOnly.length; current[b.tag] = { registryOnly: registryOnly.slice().sort(), missing: false }; const status = specOnly.length === 0 ? '✓' : '⚠'; console.log( - `${status} <${b.tag}> (${b.schemaType}): ${declaredByBoth.length} declared by both, ${specOnly.length} spec-only, ${registryOnly.length} registry-only`, + `${status} <${b.tag}> (${b.schemaType}): ${declaredByBoth.length} declared by both, ${specOnly.length} spec-only, ${registryOnly.length} registry-only, ${nodeLevel.length} node-level`, ); if (specOnly.length) console.log(` spec declares, registry does not: ${specOnly.join(', ')}`); + if (nodeLevel.length) console.log(` ${NODE_LEVEL_ACCEPT_LABEL}: ${nodeLevel.sort().join(', ')}`); if (registryOnly.length) console.log(` registry declares, spec does not: ${registryOnly.join(', ')}`); } // ── SDUI object-* blocks (#7751) — same comparison, same ratchet ───────────── @@ -278,15 +410,17 @@ for (const type of SDUI_OBJECT_BLOCK_TYPES) { } const inputSet = new Set(inputs); const specOnly = [...spec].filter((p) => !inputSet.has(p)); - const registryOnly = [...inputSet].filter((p) => !spec.has(p)); + const nodeLevel = [...inputSet].filter((p) => NODE_CONTRACT_KEYS.has(p) && !spec.has(p)); + const registryOnly = [...inputSet].filter((p) => !spec.has(p) && !NODE_CONTRACT_KEYS.has(p)); const declaredByBoth = [...spec].filter((p) => inputSet.has(p)); totalSpecOnly += specOnly.length; current[type] = { registryOnly: registryOnly.slice().sort(), missing: false }; const status = specOnly.length === 0 ? '✓' : '⚠'; console.log( - `${status} ${type}: ${declaredByBoth.length} declared by both, ${specOnly.length} spec-only, ${registryOnly.length} registry-only`, + `${status} ${type}: ${declaredByBoth.length} declared by both, ${specOnly.length} spec-only, ${registryOnly.length} registry-only, ${nodeLevel.length} node-level`, ); if (specOnly.length) console.log(` spec declares, registry does not: ${specOnly.join(', ')}`); + if (nodeLevel.length) console.log(` ${NODE_LEVEL_ACCEPT_LABEL}: ${nodeLevel.sort().join(', ')}`); if (registryOnly.length) console.log(` registry declares, spec does not: ${registryOnly.join(', ')}`); }