From df97bc941d449e0fe8661f4156f1533d9fd776ef Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 19:13:34 +0000 Subject: [PATCH 1/2] fix(sdui-parser): refuse an authored `type` attribute on the html tier (#13957) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a `kind:'html'` page the tag name IS the node's `type`, so a `type` attribute is a name collision with the envelope's discriminator. The parser now refuses it at parse time with one `forbidden-attr` diagnostic naming both the tag and the attribute, replacing two outcomes: the silent one (the value named another registered type, so the manifest resolved it and a different component rendered with zero diagnostics) and the misdirected one (the value named nothing registered, so `unknown-component` blamed the value and read as a missing plugin). `parseElement` also builds the node as `{ ...props, type: tag }` — defense in depth, correct only because the attribute is now refused loudly. `validate.ts`'s `BASE_PROPS` is untouched, no `specType` alias is introduced, and there is no warning grace period. Maintainer ruling 2026-09-01, recorded as an append-only amendment on ADR-0080. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WLJQhde67SeTccsmnBVarV --- .../sdui-parser-type-attribute-refusal.md | 36 ++++++ content/docs/ui/react-pages.mdx | 8 +- docs/adr/0080-ai-authored-ui-jsx-source.md | 35 ++++++ .../type-attribute-collision.test.ts | 108 ++++++++++++++++++ packages/sdui-parser/src/parse.ts | 71 +++++++++++- .../packages__sdui-parser__src__parse.ts.json | 7 ++ 6 files changed, 260 insertions(+), 5 deletions(-) create mode 100644 .changeset/sdui-parser-type-attribute-refusal.md create mode 100644 packages/sdui-parser/src/__tests__/type-attribute-collision.test.ts create mode 100644 scripts/adr-anchors/packages__sdui-parser__src__parse.ts.json diff --git a/.changeset/sdui-parser-type-attribute-refusal.md b/.changeset/sdui-parser-type-attribute-refusal.md new file mode 100644 index 0000000000..8654499782 --- /dev/null +++ b/.changeset/sdui-parser-type-attribute-refusal.md @@ -0,0 +1,36 @@ +--- +'@objectstack/sdui-parser': minor +--- + +html tier: an authored `type=` attribute is now refused at parse time instead of overwriting the component discriminator + +On a `kind:'html'` page the tag name **is** the node's `type`, so a `type` attribute is a +name collision with the envelope's own discriminator. The parser now refuses it with one +`forbidden-attr` error naming **both** the tag and the attribute — *Attribute "type" is +not allowed on `` — on this tier the tag name IS the component…* — replacing two +outcomes, neither good: + +- the value named another **registered** type (``): the tree carried the + author's value as its discriminator, `validateTree` resolved `grid` in the manifest, + every check passed, and the page rendered a grid where the author wrote a flex — **zero + diagnostics**, on the one tier whose premise is that unreviewed, AI-authored source is + safe to accept; +- the value named **nothing** registered (``, the shape a + react-tier author carries across): `unknown-component` naming `"bar"`, which reads as a + missing plugin rather than as an attribute that should not be there. + +Alongside the refusal, `parseElement` builds the node as `{ ...props, type: tag }` rather +than `{ type: tag, ...props }` — defense in depth, and correct only *because* the +attribute is refused loudly: reversing the spread alone would trade a silent overwrite for +a silent discard. + +The react tier is unaffected: its `specType` rescue (objectui#2880) stays where it lives +and is deliberately **not** carried over — the two tiers are two source formats, and a +consumer-side alias on a second tier is the tolerance ADR-0080's amendment declined. +`validate.ts`'s `BASE_PROPS` is unchanged (`type` is correct there for every other +member), and no warning grace period is introduced. + +**This narrows what the html tier accepts**: a page that compiles today with a `type=` +attribute will be refused. The in-repo migration surface was measured before the change +and is **zero** — no html-tier page source under `content/docs/**` or the example apps +carries one. Maintainer ruling 2026-09-01, recorded as an amendment on ADR-0080. diff --git a/content/docs/ui/react-pages.mdx b/content/docs/ui/react-pages.mdx index fa380d000c..288292e212 100644 --- a/content/docs/ui/react-pages.mdx +++ b/content/docs/ui/react-pages.mdx @@ -131,8 +131,12 @@ One collision is worth knowing. `type` is the SDUI envelope's component discrimi **and** a legitimate prop name on some blocks — a chart's family, for instance. The discriminator wins the `type` slot and your value is preserved beside it as `specType` for the block to read, so `` works as written. That rescue is the -react runtime's. On an `html` page the tag name *is* the node's `type`, a `type` attribute -overwrites it, and `` declares no `type` input to write in the first place. +react runtime's, and it stays there. On an `html` page the tag name *is* the node's +`type`, so a `type` attribute is a name collision, and the parser **refuses** it rather +than resolving it either way: `` fails to compile with *Attribute +"type" is not allowed on ``* — one diagnostic naming both the tag and the attribute. +Write the tag of the component you mean; `` declares no `type` input to +write in the first place. ### `Block` — the escape hatch diff --git a/docs/adr/0080-ai-authored-ui-jsx-source.md b/docs/adr/0080-ai-authored-ui-jsx-source.md index 96247e2f10..0c858af287 100644 --- a/docs/adr/0080-ai-authored-ui-jsx-source.md +++ b/docs/adr/0080-ai-authored-ui-jsx-source.md @@ -146,3 +146,38 @@ Coupling: completed `inputs` → codegen `.d.ts` (author-time) **and** serialize 1. Collapse the object collection-views into one `object-view` + `viewType` enum, or keep `object-kanban`/`object-calendar`/… as distinct named blocks (better AI recall vs. smaller vocabulary)? 2. Converge objectui's `${}` evaluator onto the framework CEL (ADR-0058) for the JSX-source expression layer — required for typed expressions, or deferred? 3. `compiledTree` persisted at save, or compiled lazily on read with a source-hash cache? + +--- + +> **Amendment (2026-09-01 — the `type` attribute is refused on the html tier).** An +> authored `type=` attribute on an `html`-tier element is a **name collision with the +> envelope's own discriminator**, which on this tier the tag name sets, and the parser +> **refuses it at parse time** with one diagnostic naming both the tag and the attribute. +> Maintainer ruling of 2026-09-01, quoted verbatim and untranslated: +> +> > **(b) 裁「响亮拒绝」**:作者在元素上写 `type=` 属性 = 与信封判别符的名字冲突,parser 当场一条诊断**同时点名标签与属性**(两支合一…都被这条诊断替代);⛔ `specType` 别名不引入 html tier;⛔ 不设 warning 宽限期(不考虑渐进) +> +> That one diagnostic replaces two outcomes, and the **silent** one is why the ruling went +> this way: when the authored value named another *registered* type (``) +> the tree carried the author's value as its discriminator, every manifest check passed, +> and the page rendered a different component with **zero diagnostics** — on the one tier +> whose stated premise (§2, §5) is that unreviewed, AI-authored source is safe to accept. +> When it named nothing registered, the diagnostic was `unknown-component` naming the +> *value*, which reads as a missing plugin rather than as an attribute that should not be +> there. +> +> Three boundaries the ruling drew, recorded because each was a live alternative: +> ⛔ the react tier's `specType` rescue (objectui#2880) is **not** carried over here — it +> is consumer-side tolerance, and it would spread an alias concept to a second tier +> (Prime Directive #12); the two tiers are two source formats, so refusing at the html +> tier's door does not disturb that rescue where it lives. ⛔ **No warning grace period** +> — the accept-set narrows in one payment. ⛔ The fix is **not** at the warning layer: +> `type` sits in `validate.ts`'s `BASE_PROPS` deliberately (it is correct for every other +> member) and stays there. Alongside the refusal, `parseElement` now builds the node as +> `{ ...props, type: tag }` rather than `{ type: tag, ...props }` — defense in depth, and +> correct **only** because the attribute is refused loudly: reversing the spread alone +> would trade a silent overwrite for a silent discard. +> +> **Migration surface: none.** The census the ruling ordered first (scan `content/docs/**` +> and the example apps for html-tier sources carrying `type=`) measured **zero** +> occurrences; every `type=` in those trees is react-tier or plain-HTML illustration. diff --git a/packages/sdui-parser/src/__tests__/type-attribute-collision.test.ts b/packages/sdui-parser/src/__tests__/type-attribute-collision.test.ts new file mode 100644 index 0000000000..97d3ea6033 --- /dev/null +++ b/packages/sdui-parser/src/__tests__/type-attribute-collision.test.ts @@ -0,0 +1,108 @@ +/** + * html tier: an authored `type=` attribute collides with the envelope's own + * discriminator, and is REFUSED at parse time (objectstack#13957, maintainer + * ruling 2026-09-01 — ADR-0080 amendment). + * + * The two outcomes this replaces are pinned side by side on purpose, because + * they are the reason the refusal is at parse rather than at the warning layer: + * one of them produced NO diagnostic at all, and the other produced a loud one + * pointing somewhere else. A test that only asserted "an error is raised" would + * pass on the second case before this change. + */ +import { describe, expect, it } from 'vitest'; +import { compile, manifestFromConfigs } from '../index.js'; +import { parseJsx } from '../parse.js'; + +const manifest = manifestFromConfigs([ + { type: 'flex', namespace: 'ui', isContainer: true, inputs: [ + { name: 'direction', type: 'enum', enum: ['row', 'col'] }, + { name: 'gap', type: 'number' }, + ] }, + { type: 'grid', namespace: 'ui', isContainer: true, inputs: [{ name: 'columns', type: 'number' }] }, + { type: 'object-chart', namespace: 'plugin-charts', isContainer: false, inputs: [ + { name: 'objectName', type: 'string', binding: 'object' }, + ] }, +]); + +/** The refusal, as the author sees it: one diagnostic naming BOTH names. */ +const refusals = (source: string) => + compile(source, manifest).diagnostics.filter((d) => d.code === 'forbidden-attr'); + +describe('an authored `type` attribute is refused (the discriminator collision)', () => { + it('refuses when the value names ANOTHER REGISTERED type — the previously SILENT case', () => { + const r = compile('', manifest); + + // Before this change: `grid` resolved in the manifest, every check passed, + // and the page rendered a grid where the author wrote a flex — zero + // diagnostics of any severity. + expect(r.ok).toBe(false); + expect(r.diagnostics).toContainEqual( + expect.objectContaining({ severity: 'error', code: 'forbidden-attr', tag: 'flex' }), + ); + + // ONE diagnostic, naming BOTH the tag and the attribute (the ruled shape). + const [only, ...rest] = refusals(''); + expect(rest).toEqual([]); + expect(only.message).toContain('"type"'); + expect(only.message).toContain(''); + }); + + it('refuses when the value names NOTHING registered — the previously MISDIRECTED case', () => { + // `` is the shape a react-tier author carries + // across: on that tier `type` is the chart family. Before this change the + // only diagnostic was `unknown-component` naming `"bar"`, which reads as a + // missing plugin rather than as an attribute that should not be there. + const r = compile('', manifest); + + expect(r.ok).toBe(false); + expect(refusals('')).toHaveLength(1); + expect(r.diagnostics.map((d) => d.code)).not.toContain('unknown-component'); + }); + + it('refuses a BARE `type` attribute too — the check is on the name, not the value', () => { + expect(refusals('')).toHaveLength(1); + expect(refusals('')).toHaveLength(1); + }); + + it('refuses it on a NESTED element, not only on the root', () => { + const found = refusals(''); + expect(found).toHaveLength(1); + expect(found[0].tag).toBe('grid'); + expect(found[0].message).toContain(''); + }); + + it('leaves a clean element alone — no regression', () => { + const r = compile('', manifest); + expect(r.ok).toBe(true); + expect(r.diagnostics).toEqual([]); + expect(r.tree).toMatchObject({ type: 'flex', direction: 'row', gap: 4 }); + }); +}); + +describe('spread order — defense in depth behind the refusal', () => { + it('keeps the TAG as the discriminator even when a `type` attribute was authored', () => { + // Parser-level, with no manifest: the refusal is a diagnostic, and the tree + // is still built. What must never happen is the tree carrying the AUTHOR's + // value as its discriminator — that is the silent redirect itself. + const { tree, diagnostics } = parseJsx(''); + + expect(tree?.type).toBe('flex'); + expect(diagnostics.map((d) => d.code)).toContain('forbidden-attr'); + }); + + it('and does not let the refused value land under its own name either', () => { + const { tree } = parseJsx(''); + expect(Object.values(tree ?? {})).not.toContain('grid'); + }); + + it('every other prop still survives the reordered spread', () => { + const { tree } = parseJsx(''); + expect(tree).toMatchObject({ + type: 'flex', + direction: 'col', + gap: 8, + wrap: true, + children: [{ type: 'grid', columns: 3 }], + }); + }); +}); diff --git a/packages/sdui-parser/src/parse.ts b/packages/sdui-parser/src/parse.ts index b832904b61..d44c113e15 100644 --- a/packages/sdui-parser/src/parse.ts +++ b/packages/sdui-parser/src/parse.ts @@ -20,6 +20,40 @@ import type { Diagnostic, ParseOptions, ParseResult, SchemaElement, SchemaNode } const EVENT_ATTR = /^on[A-Z]/; const FORBIDDEN_ATTRS = new Set(['dangerouslySetInnerHTML', 'ref', 'key']); +/** + * The envelope's own discriminator, which on THIS tier the tag name sets. + * + * An authored `type=` attribute is a NAME COLLISION with it, and the parser + * refuses it at parse time (maintainer ruling 2026-09-01, recorded as an + * amendment on ADR-0080 — 「响亮拒绝」, quoted verbatim there). One diagnostic + * naming BOTH the tag and the attribute replaces two bad outcomes: + * + * - the value named another REGISTERED type (``) — the tree + * carried `type:'grid'`, `validateTree` found `grid` in the manifest, every + * check passed, and the page rendered a grid where the author wrote a flex. + * ZERO diagnostics. On the one tier whose whole premise is that unreviewed + * and AI-authored source is safe to accept. + * - the value named NOTHING registered (``, the shape + * a react-tier author carries across) — loud, but `unknown-component` + * naming `"bar"` reads as a missing plugin, never as a bad prop. + * + * ⛔ NOT rescued as `specType` the way the react tier rescues it (objectui#2880): + * that is consumer-side tolerance, and it would spread an alias concept to a + * second tier. ⛔ NOT a warning grace period either — the same ruling declined + * a staged rollout. ⛔ And NOT fixable at the warning layer: `type` is in + * `validate.ts`'s `BASE_PROPS` deliberately (it is correct for every other + * member), so removing it there would make every legitimate node warn. The + * refusal belongs here, at parse. + * + * ⚠️ The code is the EXISTING `forbidden-attr`, not a new one, and that is + * load-bearing rather than lazy: `scripts/check-sdui-lockstep.mjs` holds this + * copy's diagnostic-code set equal to objectui's at the pinned revision, so a + * code minted on one side only IS the dialect split that gate exists to catch + * (#12719). `forbidden-attr` already carries this shape — an attribute this + * tier refuses, named beside its element — and both copies stamp it. + */ +const DISCRIMINATOR_ATTR = 'type'; + export function parseJsx(source: string, options: ParseOptions = {}): ParseResult { return new Parser(source, options).parseDocument(); } @@ -69,7 +103,15 @@ class Parser { if (c === '' || c === '>' || c === '/') break; const attr = this.parseAttr(start, tag); if (!attr) break; - props[attr.name] = attr.value; + // `drop` is set only for the refused discriminator attribute, and only so + // that ONE diagnostic is what the author gets. The `__forbidden_` + // sentinel the other refusals park in `props` reaches `validateTree`, + // which knows no such prop and adds `unknown-prop` naming a key nobody + // wrote — loud, and pointing at the wrong thing, which is the species of + // diagnostic this whole change exists to remove. The existing sentinel + // behaviour is left exactly as it was for the attributes that already had + // it (`ref`, `key`, `dangerouslySetInnerHTML`, `on*`). + if (!attr.drop) props[attr.name] = attr.value; } this.skipWs(); @@ -82,12 +124,22 @@ class Parser { this.error('unterminated-open-tag', `Unterminated <${tag}> open tag`, start, tag); } - const node: SchemaElement = { type: tag, ...props }; + // DEFENSE IN DEPTH (ruled together with the refusal above). `props` used to + // be spread AFTER `type: tag`, so an authored `type` attribute overwrote the + // discriminator the tag established and nothing downstream restored it — + // `compile()` returns this tree as-is and `validateTree` then looks up + // `manifest.components[node.type]`, i.e. the value the author wrote, not the + // tag they wrote. The refusal makes that overwrite unreachable; the order + // here makes it impossible. ⚠️ Reversing the order ALONE would have been a + // regression of its own — the authored value would then be dropped in + // silence, trading one silence for another. It is correct only BECAUSE the + // attribute is refused loudly one function up. + const node: SchemaElement = { ...props, type: tag }; if (children && children.length) node.children = children; return node; } - private parseAttr(elStart: number, tag: string): { name: string; value: unknown } | null { + private parseAttr(elStart: number, tag: string): { name: string; value: unknown; drop?: boolean } | null { const name = this.readName(); if (!name) { this.error('bad-attr', `Malformed attribute on <${tag}>`, this.pos, tag); @@ -101,6 +153,19 @@ class Parser { this.skipWs(); value = this.parseAttrValue(tag); } + if (name === DISCRIMINATOR_ATTR) { + // ONE diagnostic naming both the tag and the attribute — see + // DISCRIMINATOR_ATTR above for why it replaces both prior outcomes. + this.error( + 'forbidden-attr', + `Attribute "${DISCRIMINATOR_ATTR}" is not allowed on <${tag}> — on this tier the tag name IS the ` + + `component, so <${tag}> already means type "${tag}". Delete the attribute, or write the tag of the ` + + 'component you meant.', + elStart, + tag, + ); + return { name, value: undefined, drop: true }; + } if (EVENT_ATTR.test(name) || FORBIDDEN_ATTRS.has(name)) { this.error('forbidden-attr', `Attribute "${name}" is not allowed on <${tag}>`, elStart, tag); return { name: `__forbidden_${name}`, value: undefined }; diff --git a/scripts/adr-anchors/packages__sdui-parser__src__parse.ts.json b/scripts/adr-anchors/packages__sdui-parser__src__parse.ts.json new file mode 100644 index 0000000000..d48fe321c1 --- /dev/null +++ b/scripts/adr-anchors/packages__sdui-parser__src__parse.ts.json @@ -0,0 +1,7 @@ +{ + "file": "packages/sdui-parser/src/parse.ts", + "adrs": [ + "ADR-0080" + ], + "invariant": "ADR-0080 (2026-09-01 amendment) — an authored `type=` attribute on an html-tier element is a NAME COLLISION with the envelope's discriminator, which on this tier the tag name sets, and it is REFUSED at parse with one diagnostic naming both the tag and the attribute. Two things here look local and are not. (1) The refusal is not \"an attribute we happen to dislike\": before it, an authored `type` naming another REGISTERED component (``) overwrote the discriminator, resolved cleanly against the manifest, and rendered a different component with ZERO diagnostics — on the one tier whose premise is that unreviewed, AI-authored source is safe to accept; when it named nothing registered, the only diagnostic was `unknown-component` naming the VALUE, which reads as a missing plugin rather than as a bad prop. ⛔ Do not soften this to a warning, and ⛔ do not import the react tier's `specType` rescue (objectui#2880) — the ruling declined both by name, the second because a consumer-side alias would spread that concept to a second tier (Prime Directive #12). ⛔ The remedy is also NOT at the warning layer: `type` is in validate.ts's BASE_PROPS deliberately, correct for every other member, and stays. (2) `{ ...props, type: tag }` is the ruled defense-in-depth half, and its order is correct ONLY because the attribute is refused loudly one function up — reversing the spread on its own trades a silent overwrite for a silent discard. The refusal stamps the EXISTING `forbidden-attr` code rather than a new one: `scripts/check-sdui-lockstep.mjs` holds this copy's diagnostic-code set equal to objectui's at the pinned revision, so a code minted on one side only is the dialect split that gate exists to catch (#12719)." +} From 6dff387193f1edfe70bf605a10fc5d3e32e1c1e9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 19:37:26 +0000 Subject: [PATCH 2/2] test(sdui-parser): record that the spread-order half is unobservable behind the refusal Measured by ablation on the committed tree: restoring `{ type: tag, ...props }` while leaving the refusal in place keeps all eight tests green, because the refused attribute never reaches `props`. Removing the refusal turns 5 red, and removing both halves turns 6 red. Stated in the test file so a green suite is not read as proof the spread order is load-bearing on its own. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01WLJQhde67SeTccsmnBVarV --- .../__tests__/type-attribute-collision.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) 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 97d3ea6033..40b9fa898e 100644 --- a/packages/sdui-parser/src/__tests__/type-attribute-collision.test.ts +++ b/packages/sdui-parser/src/__tests__/type-attribute-collision.test.ts @@ -79,6 +79,23 @@ describe('an authored `type` attribute is refused (the discriminator collision)' }); }); +/** + * ⚠️ Read this before trusting the block below as coverage of the spread order. + * + * The order fix is DEFENSE IN DEPTH, which means the refusal makes it + * unobservable through the public API: measured by ablation on the committed + * tree, restoring `{ type: tag, ...props }` while LEAVING the refusal in place + * keeps all eight of these tests GREEN, because the refused attribute never + * reaches `props` to be spread. What turns them red is removing the refusal + * (5 red) and, additionally, the two spread-order assertions here when BOTH + * halves are removed together (6 red). + * + * So these tests pin the discriminator's identity, not the statement that + * produces it. That is not a gap to paper over with a stronger-sounding + * assertion — it is the ruled relationship between the two halves («拒绝使覆盖 + * 不可达,顺序修复是防御纵深»), and it is stated here so the next author does + * not read a green suite as proof the order is load-bearing on its own. + */ describe('spread order — defense in depth behind the refusal', () => { it('keeps the TAG as the discriminator even when a `type` attribute was authored', () => { // Parser-level, with no manifest: the refusal is a diagnostic, and the tree