diff --git a/.changeset/19969-sdui-containment-declared-children.md b/.changeset/19969-sdui-containment-declared-children.md new file mode 100644 index 0000000000..38908932d7 --- /dev/null +++ b/.changeset/19969-sdui-containment-declared-children.md @@ -0,0 +1,16 @@ +--- +'@objectstack/sdui-parser': patch +--- + +fix(sdui-parser): `not-a-container` is decided by the declared `children` input, not `isContainer`. This matches the renderer's copy of the parser (#19969) + +The save gate's `validateTree` now warns `not-a-container` on a child list only when the component's manifest entry declares no input named `children`. It never reads `isContainer`, which now means layout containment only, and it has no fallback to it. This is a port of objectui#9910 (objectui `5ea623ea`), which is already inside the pinned console build. Saving and rendering now reach the same verdict on every page again. + +Against the served `sdui.manifest.json`, five of its 59 components change: + +- `badge`, `alert`, `button` declare a `children` slot and are not flagged `isContainer`. A child list under them no longer draws a false `not-a-container` warning. +- `page:tabs`, `page:accordion` are flagged `isContainer` but declare no `children` input. They render `items[].children`, never `schema.children`, so a child list under them now draws the warning the flag used to silence. + +Clause-②: no + +`not-a-container` stays a `warning`, so the default save gate refuses no page it accepted before and accepts no page it refused. Only the two modes that treat warnings as errors see the five-component change: `os validate --strict` and `os lint --strict`. Both run `validateJsxPages`. The package's public entry adds no export. diff --git a/packages/lint/src/validate-jsx-pages.test.ts b/packages/lint/src/validate-jsx-pages.test.ts index 9ba2e721fa..ca3c594c45 100644 --- a/packages/lint/src/validate-jsx-pages.test.ts +++ b/packages/lint/src/validate-jsx-pages.test.ts @@ -36,7 +36,10 @@ describe('validateJsxPages (ADR-0080 build gate)', () => { describe('validateJsxPages — full validation with a manifest', () => { const manifest = { components: { - flex: { type: 'flex', namespace: 'ui', isContainer: true, inputs: [] }, + // Containment is the declared `children` input, not `isContainer` + // (objectui#9910, ported to the save gate by #19969) — the flag stays as + // the layout fact it is. + flex: { type: 'flex', namespace: 'ui', isContainer: true, inputs: [{ name: 'children', type: 'slot' }] }, 'object-table': { type: 'object-table', namespace: 'plugin-grid', inputs: [{ name: 'object', type: 'string', required: true }] }, }, }; diff --git a/packages/sdui-parser/objectui-lockstep.json b/packages/sdui-parser/objectui-lockstep.json index e1110df6dc..d79d906f38 100644 --- a/packages/sdui-parser/objectui-lockstep.json +++ b/packages/sdui-parser/objectui-lockstep.json @@ -56,5 +56,15 @@ "unterminated-brace", "unterminated-open-tag", "unterminated-string" - ] + ], + "containmentPredicate": { + "file": "packages/sdui-parser/src/validate.ts", + "code": "not-a-container", + "reaches": [ + "CHILD_LIST_KEY", + "acceptsChildren" + ], + "blob": "76c18fb95d1fd04223e3d18deea77223408cc0a8", + "text": "if (node . children ?. length && ! acceptsChildren ( comp ))\nexport const CHILD_LIST_KEY = 'children' ;\nexport function acceptsChildren ( comp : Pick < ManifestComponent , 'inputs' > ) : boolean { return comp . inputs . some ( ( input ) => input . name === CHILD_LIST_KEY ) ; }\n" + } } diff --git a/packages/sdui-parser/src/__tests__/compile.test.ts b/packages/sdui-parser/src/__tests__/compile.test.ts index 6a2ea8cbfd..935bafd5e5 100644 --- a/packages/sdui-parser/src/__tests__/compile.test.ts +++ b/packages/sdui-parser/src/__tests__/compile.test.ts @@ -2,14 +2,18 @@ import { describe, expect, it } from 'vitest'; import { compile, generateDts, manifestFromConfigs } from '../index.js'; // A tiny public-tier manifest, shaped exactly like getAllConfigs() output. +// Containment is the declared `children` slot input, not `isContainer` +// (objectui#9910) — the flag stays on the fixtures as the layout fact it is. const manifest = manifestFromConfigs([ { type: 'flex', namespace: 'ui', isContainer: true, inputs: [ { name: 'direction', type: 'enum', enum: ['row', 'col'] }, { name: 'gap', type: 'number' }, { name: 'wrap', type: 'boolean' }, + { name: 'children', type: 'slot' }, ] }, { type: 'card', namespace: 'ui', isContainer: true, inputs: [ { name: 'title', type: 'string' }, + { name: 'children', type: 'slot' }, ] }, { type: 'object-table', namespace: 'plugin-grid', isContainer: false, inputs: [ { name: 'object', type: 'string', required: true, binding: 'object' }, diff --git a/packages/sdui-parser/src/__tests__/containment-declared-slot.test.ts b/packages/sdui-parser/src/__tests__/containment-declared-slot.test.ts new file mode 100644 index 0000000000..8b669f76a1 --- /dev/null +++ b/packages/sdui-parser/src/__tests__/containment-declared-slot.test.ts @@ -0,0 +1,286 @@ +/** + * Containment is decided by the declared `children` input, and by NOTHING + * else — the save-gate port (#19969) of objectui#9910 Q1-A. + * + * The renderer's copy of this parser (objectui `packages/sdui-parser`, re-cut + * `5ea623ea`, inside the console pin) moved `not-a-container` off the + * `isContainer` flag onto the registration's declared `children` input. This + * copy runs in the SAVE GATE; if it kept reading the flag, the two would give + * opposite verdicts on the same page (#12719). The 2×2 (flag × slot) below is + * the same reading objectui pins in its own + * `containment-declared-slot-9910.test.ts`, because the two failure modes are + * OPPOSITE: a flag-only cell drawing nothing is the fallback silently + * re-introduced, and a slot-only cell drawing the warning is the defect. + * + * The second half uses the REAL served entries: in the tracked + * `sdui.manifest.json` (regenerated at the console pin, `0bf85eaa`) exactly + * five of 59 components disagree between the flag and the declared input. + * They are copied verbatim below rather than read from the repo root, so this + * package's tests keep reading only this package. + */ +import { describe, expect, it } from 'vitest'; +import { manifestFromConfigs, validateTree } from '../index.js'; +import { CHILD_LIST_KEY, acceptsChildren } from '../validate.js'; +import type { Manifest, ManifestComponent, SchemaElement } from '../types.js'; + +const CONTAINMENT = 'not-a-container'; +const SLOT = { name: 'children', type: 'slot' } as const; + +const containment = (node: SchemaElement, manifest: Manifest): string[] => + validateTree(node, manifest) + .diagnostics.map((d) => d.code) + .filter((code) => code === CONTAINMENT); + +/** A child list of one string node: strings are never validated, so the only + * diagnostic this can draw is the containment one on its parent. */ +const withChildren = (type: string): SchemaElement => ({ type, children: ['measured'] }); + +describe('#19969 — the save gate reads the declared `children` input, not the flag (2×2)', () => { + const manifest: Manifest = manifestFromConfigs([ + // flag, no slot — the `page:tabs` shape + { type: 'flag-only', namespace: 'ui', isContainer: true, inputs: [{ name: 'items', type: 'array' }] }, + // slot, no flag — the `button` / `badge` shape + { type: 'slot-only', namespace: 'ui', inputs: [{ name: 'label', type: 'string' }, SLOT] }, + // both — the `flex` shape + { type: 'both', namespace: 'ui', isContainer: true, inputs: [{ name: 'gap', type: 'number' }, SLOT] }, + // neither — a leaf + { type: 'neither', namespace: 'ui', inputs: [{ name: 'content', type: 'string' }] }, + // the page kinds publish `children` typed `array`: the NAME is the declaration + { type: 'array-typed', namespace: 'ui', inputs: [{ name: 'children', type: 'array', of: 'object' }] }, + ]); + + it('the predicate reads exactly the input NAMED `children`, whatever its coarse type', () => { + expect(CHILD_LIST_KEY).toBe('children'); + expect(acceptsChildren({ inputs: [SLOT] })).toBe(true); + expect(acceptsChildren({ inputs: [{ name: 'children', type: 'array' }] })).toBe(true); + expect(acceptsChildren({ inputs: [{ name: 'body', type: 'slot' }] })).toBe(false); + expect(acceptsChildren({ inputs: [] })).toBe(false); + }); + + it('⛔ the flag is NOT a fallback: flag without slot draws `not-a-container`', () => { + expect(containment(withChildren('flag-only'), manifest)).toEqual([CONTAINMENT]); + }); + + it('the slot alone is sufficient: slot without flag draws nothing', () => { + expect(containment(withChildren('slot-only'), manifest)).toEqual([]); + }); + + it('both declared draws nothing; neither declared draws the warning, at warning severity', () => { + expect(containment(withChildren('both'), manifest)).toEqual([]); + const found = validateTree(withChildren('neither'), manifest).diagnostics.filter( + (d) => d.code === CONTAINMENT, + ); + expect(found.map((d) => [d.code, d.severity, d.tag])).toEqual([[CONTAINMENT, 'warning', 'neither']]); + }); + + it('an `array`-typed `children` input counts — the page kinds', () => { + expect(containment(withChildren('array-typed'), manifest)).toEqual([]); + }); + + it('no child list, no verdict: an empty or absent list draws nothing anywhere', () => { + for (const type of ['flag-only', 'slot-only', 'both', 'neither', 'array-typed']) { + expect(containment({ type }, manifest)).toEqual([]); + expect(containment({ type, children: [] }, manifest)).toEqual([]); + } + }); + + it('a component the manifest does not know draws `unknown-component`, never a containment verdict', () => { + const codes = validateTree(withChildren('nowhere'), manifest).diagnostics.map((d) => d.code); + expect(codes).toEqual(['unknown-component']); + }); +}); + +describe('#19969 — the five served entries where the flag and the declared input disagree', () => { + // Verbatim from the tracked `sdui.manifest.json` at `0bf85eaa`. + const SERVED: Record = { + "badge": { + "type": "badge", + "namespace": "ui", + "inputs": [ + { + "name": "label", + "type": "string" + }, + { + "name": "variant", + "type": "enum", + "enum": [ + "default", + "secondary", + "destructive", + "outline" + ] + }, + { + "name": "className", + "type": "string" + }, + { + "name": "children", + "type": "slot", + "description": "Rich label content, rendered when `label` is not set" + } + ] + }, + "alert": { + "type": "alert", + "namespace": "ui", + "inputs": [ + { + "name": "title", + "type": "string", + "required": true + }, + { + "name": "description", + "type": "string" + }, + { + "name": "variant", + "type": "enum", + "enum": [ + "default", + "destructive" + ] + }, + { + "name": "className", + "type": "string" + }, + { + "name": "children", + "type": "slot", + "description": "Rich description content, rendered when `description` is not set" + } + ] + }, + "button": { + "type": "button", + "namespace": "ui", + "inputs": [ + { + "name": "label", + "type": "string" + }, + { + "name": "variant", + "type": "enum", + "enum": [ + "default", + "secondary", + "destructive", + "outline", + "ghost", + "link" + ] + }, + { + "name": "size", + "type": "enum", + "enum": [ + "default", + "sm", + "lg", + "icon" + ] + }, + { + "name": "className", + "type": "string" + }, + { + "name": "children", + "type": "slot", + "description": "Rich label content, rendered when `label` is not set" + } + ] + }, + "page:tabs": { + "type": "page:tabs", + "namespace": "page", + "isContainer": true, + "inputs": [ + { + "name": "items", + "type": "array", + "of": "object", + "required": true, + "description": "Tab definitions [{ label, value?, icon?, count?, visibleWhen?, children }] — value is the stable ?tab= URL token, count auto-derives from record:related_list descendants when omitted" + }, + { + "name": "tabStyle", + "type": "enum", + "enum": [ + "line", + "card", + "pill" + ] + }, + { + "name": "position", + "type": "enum", + "enum": [ + "top", + "left" + ] + }, + { + "name": "alwaysShowStrip", + "type": "boolean", + "description": "Keep the tab strip visible when only one tab survives. Default false: a lone pill is clutter rather than an affordance, so a one-tab strip is hidden and its panel renders bare. Count the tabs AFTER each item visibleWhen predicate has been evaluated — a page authored with four tabs of which three are conditional reaches this rule whenever the other three are false." + } + ] + }, + "page:accordion": { + "type": "page:accordion", + "namespace": "page", + "isContainer": true, + "inputs": [ + { + "name": "items", + "type": "array", + "of": "object", + "required": true, + "description": "Panel definitions [{ label, icon?, collapsed?, children }] — collapsed: false opens a panel by default" + }, + { + "name": "allowMultiple", + "type": "boolean" + }, + { + "name": "variant", + "type": "enum", + "enum": [ + "flush", + "card" + ] + } + ] + } + } as unknown as Record; + const manifest: Manifest = { components: SERVED }; + + it('control: each entry really does disagree — the flag and the declared input answer differently', () => { + // If a regenerated manifest ever made these agree, the rows below would stop + // distinguishing the two predicates and this file would pass under either. + for (const [type, comp] of Object.entries(SERVED)) { + expect({ type, flag: Boolean(comp.isContainer) }).not.toEqual({ type, flag: acceptsChildren(comp) }); + } + }); + + it.each(['badge', 'alert', 'button'])( + '<%s> declares the slot with the flag off: its child list draws NO `not-a-container` (was a false warning)', + (type) => { + expect(SERVED[type]!.isContainer).toBeFalsy(); + expect(containment(withChildren(type), manifest)).toEqual([]); + }, + ); + + it.each(['page:tabs', 'page:accordion'])( + '<%s> carries the flag with no slot: its child list DRAWS `not-a-container` (was silenced by the flag)', + (type) => { + expect(SERVED[type]!.isContainer).toBe(true); + expect(containment(withChildren(type), manifest)).toEqual([CONTAINMENT]); + }, + ); +}); diff --git a/packages/sdui-parser/src/__tests__/type-attribute-collision.test.ts b/packages/sdui-parser/src/__tests__/type-attribute-collision.test.ts index 40b9fa898e..75b78de7a6 100644 --- a/packages/sdui-parser/src/__tests__/type-attribute-collision.test.ts +++ b/packages/sdui-parser/src/__tests__/type-attribute-collision.test.ts @@ -17,8 +17,9 @@ const manifest = manifestFromConfigs([ { type: 'flex', namespace: 'ui', isContainer: true, inputs: [ { name: 'direction', type: 'enum', enum: ['row', 'col'] }, { name: 'gap', type: 'number' }, + { name: 'children', type: 'slot' }, ] }, - { type: 'grid', namespace: 'ui', isContainer: true, inputs: [{ name: 'columns', type: 'number' }] }, + { type: 'grid', namespace: 'ui', isContainer: true, inputs: [{ name: 'columns', type: 'number' }, { name: 'children', type: 'slot' }] }, { type: 'object-chart', namespace: 'plugin-charts', isContainer: false, inputs: [ { name: 'objectName', type: 'string', binding: 'object' }, ] }, diff --git a/packages/sdui-parser/src/types.ts b/packages/sdui-parser/src/types.ts index 567bc1259b..97739b9dd7 100644 --- a/packages/sdui-parser/src/types.ts +++ b/packages/sdui-parser/src/types.ts @@ -159,7 +159,18 @@ export interface ManifestComponent { type: string; /** plugin namespace — provenance that drives `requires` */ namespace?: string; + /** + * The declared authoring surface. An input named `children` (type `slot`) + * is ALSO the containment declaration: `validateTree` draws + * `not-a-container` on a child list under any component whose `inputs` + * carry no such entry (objectui#9910; `acceptsChildren` in `validate.ts`). + */ inputs: ManifestInput[]; + /** + * LAYOUT containment (objectui#6804, objectui#9910 Q2-A): the flag the + * react-page JSX scope builder skips and the public layout ledger lists. + * ⛔ Not "accepts children" — this tier's containment check does not read it. + */ isContainer?: boolean; } diff --git a/packages/sdui-parser/src/validate.ts b/packages/sdui-parser/src/validate.ts index 156a264073..819b7b6a83 100644 --- a/packages/sdui-parser/src/validate.ts +++ b/packages/sdui-parser/src/validate.ts @@ -2,14 +2,18 @@ * ObjectUI — SDUI tree validation against the registry manifest (ADR-0080 §3/§6) * * Shallow, author-time validation: unknown component, unknown/missing prop, - * wrong coarse type, illegal enum value. Collects `requires` (plugin provenance) - * and binding sites the SERVER must resolve against object schema (we cannot - * resolve objects/fields here — that check is framework-side by design). + * wrong coarse type, illegal enum value, and containment — a child list under + * a component whose registration declares no `children` input draws + * `not-a-container` (objectui#9910; see `acceptsChildren`). Collects + * `requires` (plugin provenance) and binding sites the SERVER must resolve + * against object schema (we cannot resolve objects/fields here — that check is + * framework-side by design). */ import type { Diagnostic, Manifest, + ManifestComponent, ManifestInput, ManifestInputType, SchemaElement, @@ -20,7 +24,49 @@ import { inputTypeArms } from './input-type.js'; import { checkDashboardWidgetOptions } from './dashboard-widget-options.js'; import { checkKanbanQuickAdd } from './kanban-quick-add.js'; -/** Base props every node may carry (mirrors BaseSchema) — never "unknown prop". */ +/** + * The protocol's ONE child-list key — `BaseSchema.children` — and therefore the + * name of the `inputs` entry a registration declares when its renderer puts + * that list on the page: `{ name: 'children', type: 'slot' }` (objectui#9910). + */ +export const CHILD_LIST_KEY = 'children'; + +/** + * Does this component ACCEPT an authored child list? (objectui#9910) + * + * Read from exactly ONE declaration: an input named {@link CHILD_LIST_KEY} in + * the component's `inputs`, whatever its coarse type — the page kinds publish + * it typed `array`, the rest typed `slot`, and the NAME is the declaration. + * objectui's registry census holds that declaration true in both directions + * (a renderer that puts `schema.children` on the page declares it; a + * declaration whose renderer never renders the list is red). + * + * ⛔ `isContainer` is NOT consulted and is NOT a fallback. The flag now means + * LAYOUT containment only (objectui#9910 Q2-A) and it disagrees with the + * declared input both ways in the served manifest: `badge` / `alert` / + * `button` declare the slot with the flag off, `page:tabs` / + * `page:accordion` carry the flag with no slot. + * + * LOCKSTEP: this function, {@link CHILD_LIST_KEY} and the containment branch + * in `validateTree` are the byte-equal port of objectui's copy (objectui + * `packages/sdui-parser/src/validate.ts`, objectui#9910 re-cut `5ea623ea`), + * so the save gate and the renderer reach the same `not-a-container` verdict + * on every page (#12719). `pnpm check:sdui-lockstep` compares this predicate + * against the vendored record of objectui's and goes red when they diverge + * (#19969) — change it only together with objectui. + */ +export function acceptsChildren(comp: Pick): boolean { + return comp.inputs.some((input) => input.name === CHILD_LIST_KEY); +} + +/** + * Base props every node may carry (mirrors BaseSchema) — never "unknown prop". + * + * `children` IS here: the key is legal on every node, so it never draws + * `unknown-prop` and its declared `slot` input is never type-checked. Whether + * a given component RENDERS it is the containment question below, answered by + * {@link acceptsChildren} from the declared input. + */ const BASE_PROPS = new Set([ 'type', 'id', @@ -30,7 +76,7 @@ const BASE_PROPS = new Set([ 'visibleOn', 'disabled', 'disabledOn', - 'children', + CHILD_LIST_KEY, ]); const isExpr = (v: unknown): boolean => @@ -164,8 +210,10 @@ export function validateTree(tree: SchemaElement | null, manifest: Manifest): Va } } - // containment - if (node.children?.length && !comp.isContainer) { + // containment — decided by the declared `children` input and by NOTHING + // else (objectui#9910 Q1-A). ⛔ No `isContainer` fallback: see + // `acceptsChildren` for why the flag stopped deciding this. + if (node.children?.length && !acceptsChildren(comp)) { diagnostics.push({ severity: 'warning', code: 'not-a-container', diff --git a/scripts/check-sdui-lockstep.mjs b/scripts/check-sdui-lockstep.mjs index d125790b49..a271683a9c 100644 --- a/scripts/check-sdui-lockstep.mjs +++ b/scripts/check-sdui-lockstep.mjs @@ -44,6 +44,28 @@ * The `.objectui-sha` clause below is the one offline instrument that reaches * into the second class, and it is deliberate rather than incidental — see it. * + * ## The third comparison: the containment PREDICATE (#19969) + * + * Codes and the literal region cannot see a change in WHEN a code fires. + * objectui#9910 moved `not-a-container` off `comp.isContainer` onto the + * declared `children` input without adding or removing a code and without + * touching `parse.ts`, so both comparisons above stayed green across a change + * that made the save gate and the renderer disagree on five served + * components. So the record also carries objectui's containment predicate: + * the condition of the ONE `if` in `validate.ts` whose body stamps + * `not-a-container`, plus every module-level declaration of that file the + * condition reaches (transitively — `acceptsChildren`, then `CHILD_LIST_KEY`), + * reduced to its TOKENS (comments and whitespace dropped), so a comment or a + * re-wrap moves nothing and a changed token does. `[containment-drift]` prints + * both token texts. + * + * What it CANNOT see: how `comp` is looked up before the branch, and the body + * of a helper imported from another module (the closure stops at the file + * boundary; an imported name is still printed as written, so swapping it + * shows). A `validate.ts` with zero or two such branches REFUSES — measuring + * one of two is the silent-short-set failure the code extractor refuses for + * the same reason. + * * ## Why `--update` cannot launder a divergence * * `--update` re-reads OBJECTUI's side only. It never reads this tree's parser, @@ -144,6 +166,12 @@ const REGION_FILE = 'packages/sdui-parser/src/parse.ts'; */ const REGION_DELIMITER = '/* ---------------------- the JS literal subset (#6614) ---------------------- */'; +/** The file whose `not-a-container` branch is the containment predicate, in both copies. */ +const CONTAINMENT_FILE = 'packages/sdui-parser/src/validate.ts'; + +/** The diagnostic code whose guarding condition IS the containment predicate. */ +const CONTAINMENT_CODE = 'not-a-container'; + /** The vendored record of OBJECTUI's side. One side only — see the header. */ const RECORD_FILE = 'packages/sdui-parser/objectui-lockstep.json'; @@ -298,13 +326,119 @@ export function extractDiagnosticCodes({ files, read }) { return { codes: [...codes.keys()].sort(), sites: codes, unresolved }; } -/** One copy's whole parity fingerprint: the region reading plus the code set. */ +/** + * The containment predicate of one `validate.ts`, as canonical text plus its + * blob id, or `{ predicate: null, why }` when it cannot be read whole. + * + * The predicate is the condition of the ONE `if` whose then-branch stamps + * `code: 'not-a-container'`, closed over every module-level function or + * variable declaration of the same file that the condition names — and that + * those name in turn. A property NAME (`node.children`, `input.name`) is not a + * reference; a local (`comp`, `node`) resolves to no module-level declaration + * and is left as written. Each piece is reduced to its token sequence — the + * AST's leaf tokens joined by one space, JSDoc nodes skipped, trivia never + * reached — one line per piece, declarations in name order, so the text + * depends on tokens and not on comments, wrapping or declaration order. (The + * TypeScript printer was tried first and is not this: it keeps a source line + * break before a `.` in a call chain, so a re-wrap moved the reading.) + */ +export function extractContainmentPredicate(text, rel = CONTAINMENT_FILE) { + const sourceFile = parseSourceFile(rel, text); + const print = (root) => { + const tokens = []; + const walk = (node) => { + if (ts.isJSDoc(node)) return; + const children = node.getChildren(sourceFile); + if (children.length === 0) { + if (node.kind !== ts.SyntaxKind.EndOfFileToken) tokens.push(node.getText(sourceFile)); + return; + } + children.forEach(walk); + }; + walk(root); + return tokens.filter((t) => t !== '').join(' '); + }; + + const topLevel = new Map(); + for (const statement of sourceFile.statements) { + if (ts.isFunctionDeclaration(statement) && statement.name) topLevel.set(statement.name.text, statement); + if (ts.isVariableStatement(statement)) { + for (const decl of statement.declarationList.declarations) { + if (ts.isIdentifier(decl.name)) topLevel.set(decl.name.text, statement); + } + } + } + + const stampsContainment = (root) => { + let found = false; + const visit = (node) => { + if ( + ts.isPropertyAssignment(node) + && ts.isIdentifier(node.name) + && node.name.text === 'code' + && ts.isStringLiteralLike(node.initializer) + && node.initializer.text === CONTAINMENT_CODE + ) { + found = true; + } + if (!found) ts.forEachChild(node, visit); + }; + visit(root); + return found; + }; + + const branches = []; + const findBranches = (node) => { + if (ts.isIfStatement(node) && stampsContainment(node.thenStatement)) branches.push(node); + ts.forEachChild(node, findBranches); + }; + findBranches(sourceFile); + if (branches.length !== 1) { + return { + predicate: null, + why: `${rel} has ${branches.length} \`if\` branch(es) stamping \`${CONTAINMENT_CODE}\`, and exactly one is the predicate`, + }; + } + const condition = branches[0].expression; + + const referencedFrom = (root) => { + const names = []; + const visit = (node) => { + if (ts.isIdentifier(node)) { + const parent = node.parent; + const isPropertyName = parent + && ((ts.isPropertyAccessExpression(parent) && parent.name === node) + || (ts.isPropertyAssignment(parent) && parent.name === node)); + if (!isPropertyName && topLevel.has(node.text)) names.push(node.text); + } + ts.forEachChild(node, visit); + }; + visit(root); + return names; + }; + const reached = new Set(); + const queue = referencedFrom(condition); + while (queue.length > 0) { + const name = queue.shift(); + if (reached.has(name)) continue; + reached.add(name); + queue.push(...referencedFrom(topLevel.get(name))); + } + + const reaches = [...reached].sort(); + const printedDecls = [...new Set(reaches.map((name) => topLevel.get(name)))].map(print); + const canonical = `${[`if (${print(condition)})`, ...printedDecls].join('\n')}\n`; + return { predicate: { text: canonical, blob: gitBlobHash(canonical), reaches }, why: null }; +} + +/** One copy's whole parity fingerprint: the region reading, the code set and the containment predicate. */ export function fingerprintOf(root) { const files = parserSources(root); const read = (rel) => readFileSync(join(root, rel), 'utf8'); const region = readRegion(read(REGION_FILE)); const { codes, unresolved } = extractDiagnosticCodes({ files, read }); - return { files, region, codes, unresolved }; + const containment = extractContainmentPredicate(read(CONTAINMENT_FILE)); + return { files, region, codes, unresolved, containment }; } // ───────────────────────────────────────────────────────────────────────────── @@ -330,6 +464,13 @@ export function recordProblems(record) { if (!Array.isArray(record.diagnosticCodes) || record.diagnosticCodes.length === 0) { problems.push('`diagnosticCodes` is not a non-empty array'); } + if ( + typeof record.containmentPredicate?.blob !== 'string' + || !/^[0-9a-f]{40}$/.test(record.containmentPredicate.blob) + || typeof record.containmentPredicate?.text !== 'string' + ) { + problems.push('`containmentPredicate` does not carry a 40-character `blob` and its `text`'); + } return problems; } @@ -367,6 +508,13 @@ export function judge({ record, ours, livePin }) { + ' extractor read nothing rather than finding nothing.', ); } + if (ours.containment?.predicate == null) { + findings.push( + `[containment-unreachable] ${ours.containment?.why ?? `no containment reading was taken of ${CONTAINMENT_FILE}`}.\n` + + ' Nothing was compared. The predicate that decides `not-a-container` is part of the lockstep (#19969),\n' + + ' and a copy whose predicate cannot be located is not a copy that agrees.', + ); + } if (findings.length > 0) return findings; if (record.recordedAgainstPin !== livePin) { @@ -389,6 +537,17 @@ export function judge({ record, ours, livePin }) { ); } + if (ours.containment.predicate.blob !== record.containmentPredicate.blob) { + const indent = (t) => t.trimEnd().split('\n').map((l) => ` ${l}`).join('\n'); + findings.push( + `[containment-drift] the \`${CONTAINMENT_CODE}\` predicate in ${CONTAINMENT_FILE} is not the one objectui's copy\n` + + ` runs at ${record.objectui.rev.slice(0, 12)}, so the save gate and the renderer disagree about which\n` + + ' components accept a child list.\n' + + ` here blob ${ours.containment.predicate.blob}\n${indent(ours.containment.predicate.text)}\n` + + ` recorded blob ${record.containmentPredicate.blob}\n${indent(record.containmentPredicate.text)}`, + ); + } + const recorded = new Set(record.diagnosticCodes); const mine = new Set(ours.codes); const onlyHere = ours.codes.filter((c) => !recorded.has(c)); @@ -544,6 +703,13 @@ function update() { ); process.exit(1); } + if (theirs.containment.predicate === null) { + console.error( + `\ncheck-sdui-lockstep --update: REFUSED — objectui's containment predicate could not be read: ${theirs.containment.why}.\n` + + ' Nothing was recorded.\n', + ); + process.exit(1); + } if (theirs.unresolved.length > 0 || theirs.codes.length === 0) { console.error( '\ncheck-sdui-lockstep --update: REFUSED — objectui\'s diagnostic-code set could not be read whole:\n' @@ -577,12 +743,20 @@ function update() { blob: theirs.region.blob, }, diagnosticCodes: theirs.codes, + containmentPredicate: { + file: CONTAINMENT_FILE, + code: CONTAINMENT_CODE, + reaches: theirs.containment.predicate.reaches, + blob: theirs.containment.predicate.blob, + text: theirs.containment.predicate.text, + }, }; writeFileSync(join(ROOT, RECORD_FILE), `${JSON.stringify(record, null, 2)}\n`); console.log( `check-sdui-lockstep --update: recorded objectui@${record.objectui.rev.slice(0, 12)} ` + `(${record.objectui.revDate}) — ${record.grammarRegion.lines} region line(s), blob ` + `${record.grammarRegion.blob.slice(0, 12)}, ${record.diagnosticCodes.length} diagnostic code(s), ` + + `containment predicate ${record.containmentPredicate.blob.slice(0, 12)}, ` + `against pin ${record.recordedAgainstPin.slice(0, 12)}.`, ); console.log(` Wrote ${RECORD_FILE}. Now run \`pnpm check:sdui-lockstep\` — a RED there is the port you still owe.`); @@ -611,6 +785,33 @@ class P { export const stamp = { code: UNCONSUMED_WIDGET_OPTION, message: 'x' }; `; +// The containment fixtures: the two predicates objectui#9910 moved between, +// shaped like `validate.ts` (a branch INSIDE the walker, the helper and its +// constant at module level, comments on both). +const FIXTURE_CONTAINMENT_SLOT = ` +export const CHILD_LIST_KEY = 'children'; +/** Does it accept a child list? */ +export function acceptsChildren(comp: { inputs: Array<{ name: string }> }): boolean { + return comp.inputs.some((input) => input.name === CHILD_LIST_KEY); +} +const UNRELATED = 'not-reached'; +export function validateTree(node: any, comp: any, diagnostics: any[]): void { + // containment + if (node.children?.length && !acceptsChildren(comp)) { + diagnostics.push({ severity: 'warning', code: 'not-a-container', message: 'x' }); + } + if (node.x) diagnostics.push({ code: 'unknown-prop', message: UNRELATED }); +} +`; + +const FIXTURE_CONTAINMENT_FLAG = ` +export function validateTree(node: any, comp: any, diagnostics: any[]): void { + if (node.children?.length && !comp.isContainer) { + diagnostics.push({ severity: 'warning', code: 'not-a-container', message: 'x' }); + } +} +`; + const FIXTURE_UNRESOLVABLE = ` import { SOMEWHERE_ELSE } from './elsewhere.js'; export const stamp = { code: SOMEWHERE_ELSE, message: 'x' }; @@ -645,11 +846,12 @@ const SELF_TEST_BATTERIES = Object.freeze({ 'Absence is loud: every degraded input REFUSES rather than passing': 8, 'The generator will not record a tree the pin does not name': 7, 'The wiring this gate needs to be reachable at all': 12, + 'The containment predicate (#19969)': 10, }); // DELETING an entry silences that battery's floor exactly as effectively as // zeroing it, so the roster's own size is pinned too. -const SELF_TEST_BATTERY_FLOOR = 6; +const SELF_TEST_BATTERY_FLOOR = 7; // The key an assertion is filed under when no battery is open. It is not a // declared battery, so it reds by the same set difference rather than silently @@ -750,13 +952,21 @@ export function selfTest() { // ── The verdict ────────────────────────────────────────────────────────── battery('The verdict'); + const slotReading = extractContainmentPredicate(FIXTURE_CONTAINMENT_SLOT, 'slot.ts'); + const flagReading = extractContainmentPredicate(FIXTURE_CONTAINMENT_FLAG, 'flag.ts'); const goodRecord = { objectui: { rev: 'a'.repeat(40) }, recordedAgainstPin: 'b'.repeat(40), grammarRegion: { lines: 3, blob: region.blob }, diagnosticCodes: ['no-root', 'unconsumed-widget-option'], + containmentPredicate: { blob: slotReading.predicate?.blob, text: slotReading.predicate?.text }, + }; + const goodOurs = { + region, + codes: ['no-root', 'unconsumed-widget-option'], + unresolved: [], + containment: slotReading, }; - const goodOurs = { region, codes: ['no-root', 'unconsumed-widget-option'], unresolved: [] }; const kinds = (f) => f.map((x) => x.slice(1, x.indexOf(']'))).join(','); check('agreement is silent', judge({ record: goodRecord, ours: goodOurs, livePin: 'b'.repeat(40) }).length === 0); check( @@ -938,6 +1148,80 @@ export function selfTest() { /recordedAgainstPin: livePin,/.test(updateSrc) && !/recordedAgainstPin: readFileSync/.test(updateSrc), ); + // ── The containment predicate (#19969) ─────────────────────────────────── + // + // The comparison the code set and the region cannot make: WHEN a code fires. + // The first case is the reason the battery exists — the two predicates + // objectui#9910 moved between must read DIFFERENT, or the pin is blind to the + // very change that motivated it. + battery('The containment predicate (#19969)'); + check( + '⭐ the flag predicate and the declared-slot predicate read DIFFERENT', + slotReading.predicate !== null && flagReading.predicate !== null + && slotReading.predicate.blob !== flagReading.predicate.blob, + `${slotReading.predicate?.blob} vs ${flagReading.predicate?.blob}`, + ); + check( + 'the closure reaches the helper AND the constant the helper names, and nothing unreferenced', + slotReading.predicate?.reaches.join(',') === 'CHILD_LIST_KEY,acceptsChildren', + slotReading.predicate?.reaches.join(','), + ); + check( + 'a property NAME is not a reference (`input.name`, `node.children`), a flag read reaches nothing', + flagReading.predicate?.reaches.length === 0, + flagReading.predicate?.reaches.join(','), + ); + check( + 'comments and wrapping do not move the reading', + extractContainmentPredicate( + FIXTURE_CONTAINMENT_SLOT + .replace('/** Does it accept a child list? */', '/** Reworded entirely. */') + .replace('return comp.inputs.some((input) => input.name === CHILD_LIST_KEY);', + 'return comp.inputs\n .some((input) =>\n input.name === CHILD_LIST_KEY);'), + 'slot.ts', + ).predicate?.blob === slotReading.predicate?.blob, + ); + check( + 'a change two hops away — the constant the helper reads — moves the reading', + extractContainmentPredicate( + FIXTURE_CONTAINMENT_SLOT.replace("export const CHILD_LIST_KEY = 'children';", "export const CHILD_LIST_KEY = 'body';"), + 'slot.ts', + ).predicate?.blob !== slotReading.predicate?.blob, + ); + check( + 'a validate.ts with NO containment branch refuses, never reads as an empty predicate', + extractContainmentPredicate('export const x = 1;\n', 'none.ts').predicate === null, + ); + check( + 'a validate.ts with TWO containment branches refuses, never measures one of them', + extractContainmentPredicate(`${FIXTURE_CONTAINMENT_FLAG}\n${FIXTURE_CONTAINMENT_FLAG.replace('validateTree', 'again')}`, 'two.ts') + .predicate === null, + ); + check( + 'a predicate that differs from the record is a finding', + kinds(judge({ + record: goodRecord, + ours: { ...goodOurs, containment: flagReading }, + livePin: 'b'.repeat(40), + })) === 'containment-drift', + ); + check( + 'an unreadable predicate in THIS tree refuses instead of comparing', + kinds(judge({ + record: goodRecord, + ours: { ...goodOurs, containment: { predicate: null, why: 'two branches' } }, + livePin: 'b'.repeat(40), + })) === 'containment-unreachable', + ); + check( + 'a record without the predicate is unusable, not a pass', + kinds(judge({ + record: { ...goodRecord, containmentPredicate: undefined }, + ours: goodOurs, + livePin: 'b'.repeat(40), + })) === 'record-unusable', + ); + // ── The floor: every declared battery RAN, and ran its cases (#13489) ─── // // Evaluated after every battery has had its chance and BEFORE the verdict, so @@ -991,7 +1275,8 @@ export function selfTest() { console.log( 'check:sdui-lockstep --self-test passed (the constant-vs-literal decomposition and its unresolvable ' + 'direction, the region reader in both drift directions, all four refusal classes, the generator\'s ' - + 'HEAD-vs-pin refusal in both directions, and the CI wiring)', + + 'HEAD-vs-pin refusal in both directions, the CI wiring, and the containment predicate in both ' + + 'directions)', ); return SELF_TEST_VERDICT; @@ -1040,7 +1325,8 @@ function main() { console.log( `check:sdui-lockstep: OK — this copy is byte-identical to objectui@${record.objectui.rev.slice(0, 12)} ` + `(${record.objectui.revDate}) over ${ours.region.lines} grammar line(s) ` - + `[blob ${ours.region.blob.slice(0, 12)}] and agrees on all ${ours.codes.length} diagnostic code(s), ` + + `[blob ${ours.region.blob.slice(0, 12)}], agrees on all ${ours.codes.length} diagnostic code(s) ` + + `and on the \`${CONTAINMENT_CODE}\` predicate [blob ${ours.containment.predicate.blob.slice(0, 12)}], ` + `across ${ours.files.length} non-test source(s).`, ); console.log(