diff --git a/.changeset/23026-retired-component-prop-refused.md b/.changeset/23026-retired-component-prop-refused.md new file mode 100644 index 00000000000..d97ae066c35 --- /dev/null +++ b/.changeset/23026-retired-component-prop-refused.md @@ -0,0 +1,29 @@ +--- +'@objectstack/lint': major +--- + +A retired page-component prop is refused at `objectstack validate`, `objectstack build` and `objectstack lint`. Before this change, a key that `ComponentPropsMap` tombstones with `retiredKey()` (`object-grid` `defaultFilters`, `element:record_picker` `displayField`, `page:header` `breadcrumb` and the rest) drew only a `component-props-invalid` warning. The command printed "✓ Validation passed" and built the artifact, while the conversion registry told authors the key is refused. It is now a `component-props-invalid` error carrying the tombstone's prescription, and the stack does not pass until the key is gone. + +Clause-②: no (narrowing) + + + +**BREAKING**: an accept-set narrowing on a published authoring surface (the `component-props-invalid` rule of `objectstack validate`, `objectstack build` and `objectstack lint`). + +**What is refused.** A value verdict a tombstone raises: the tombstone's own issue, wherever the key sits in the props bag, an array entry's key included. A tombstoned key inside a union member is refused when that member is the only one judging keys (`object-form` `sections[].fields[]` `publicPicker`). A retired element's props row (`user:profile`, `ai:chat_window`) raises the same issue and is refused the same way. + +**What stays a warning.** Every other `component-props-invalid` finding: a wrong type, a missing required prop, a closed-set refusal (a url action's `target` scheme, a `record:details` section's `count`), a retired enum value and a missing `dataSource.object` binding. Every `component-props-unknown-key` finding also stays a warning. + +**Not judged here.** The runtime publish door (the Studio, REST `/meta`, MCP) does not run this rule, so a page saved there is not refused for a retired prop. `defineStack` and the stack schema keep the open `properties` bag as written. + +## FROM → TO + +| you wrote | write instead | +|:--|:--| +| `object-grid` `properties: { defaultFilters: [...] }` with an empty `filter` | `properties: { filter: [...] }` | +| `element:record_picker` `properties: { displayField: 'title' }` | `properties: { labelField: 'title' }` | +| a retired prop whose prescription says to delete it, e.g. `page:header` `breadcrumb` | nothing: delete the key | + +**The one-line fix: apply the prescription the error prints.** Where an ADR-0087 conversion covers the key, `objectstack migrate meta --from ` lists the mechanical edits for existing sources. + +**Who is affected, measured.** At objectstack `21c86444e`, this rule ran over `examples/app-crm`, `app-todo`, `app-multi-package` and `app-showcase` (through `objectstack validate`), over the five shipped pages outside them (`@objectstack/platform-objects` ×3, `@objectstack/plugin-approvals`, `packages/qa/downstream-contract`) and over the `create-objectstack` template, which authors no page. Result: 0 newly refused, and 0 findings of this rule of any kind. Deployed metadata and other repositories were not measured. diff --git a/content/docs/deployment/validating-metadata.mdx b/content/docs/deployment/validating-metadata.mdx index b4632a5b62c..b027484c316 100644 --- a/content/docs/deployment/validating-metadata.mdx +++ b/content/docs/deployment/validating-metadata.mdx @@ -470,6 +470,7 @@ orthogonal to both, and no cell here can carry it; it is written out in | Chart bindings outside dashboards (#3583) | ✓ | ✓ | ✓ | — | | Navigation vs. granted access (ADR-0090 D6) | ✓ | ✓ | ✓ | — | | SDUI scoped styling (ADR-0065) | ✓ | ✓ | ✓ | — | +| Retired SDUI component props — a key `ComponentPropsMap` tombstones (`retiredKey()`), refused with its prescription (#23026) | ✓ | ✓ | ✓ | — | | JSX / React page source parses (ADR-0080/0081) | ✓ | ✓ | ✓ | — | | Approval-node approvers (ADR-0090 D3) | ✓ | ✓ | ✓ | ✓ᶠ | | Security posture (ADR-0090 — e.g. every custom object declares `sharingModel`) | ✓ | ✓ | ✓ | ✓ˢᵖᵉᵇᵒ | @@ -484,7 +485,7 @@ orthogonal to both, and no cell here can carry it; it is written out in | Flow authoring anti-patterns (#1874) | ✓ | ✓ | ✓ | ✓ᶠ | | Flow trigger readiness — a flow that looks armed and never launches (#5762) | ✓ | ✓ | ✓ | ✓ᶠ | | `views[]` conditional-visibility predicates — CEL syntax, parse budget, bare identifiers, binding-root layer, schema path refs (ADR-0089 D3b, #7010) | ✓ | ✓ | ✓ | ✓ᵛ | -| Advisory: record titles, semantic field pointers (ADR-0085), form-section layout, action placement, SDUI component props, seed replay/state safety, capability references, liveness | ✓ | ✓ | ✓ | — | +| Advisory: record titles, semantic field pointers (ADR-0085), form-section layout, action placement, SDUI component props (every finding but a retired prop), seed replay/state safety, capability references, liveness | ✓ | ✓ | ✓ | — | | Package docs — flatness, prefixes, links (ADR-0046) | ✓ | ✓ | ✓ | — | | Undeclared authoring keys — every metadata collection (#3786) and the stack's own top-level keys (#4167) | ✓ | ✓ | — | — | | Declared-unique scope — a bare `unique: true` index, a field-level and index-level double declaration, legacy organization composites (ADR-0120 D5) | ✓ | ✓ | ✓ | — | diff --git a/content/docs/protocol/objectui/layout-dsl.mdx b/content/docs/protocol/objectui/layout-dsl.mdx index bd231457a59..e728d045fd4 100644 --- a/content/docs/protocol/objectui/layout-dsl.mdx +++ b/content/docs/protocol/objectui/layout-dsl.mdx @@ -739,9 +739,10 @@ properties: `properties` is an open bag (`Record`), so **it is the one part of a component the spec parse does not check**. For the platform's own types the - authoring rules dispatch `ComponentPropsMap` and reject a misspelled prop; a - `custom.*` type has no entry there, so its props are validated only by whatever - registered the component. + authoring rules (`os validate`, `os build`, `os lint`) dispatch `ComponentPropsMap`: + a misspelled prop is reported as a warning, and a retired prop is refused as an + error, with its prescription. A `custom.*` type has no entry there, so its props + are validated only by whatever registered the component. ## Advanced Layouts diff --git a/packages/lint/src/authoring-rules.ts b/packages/lint/src/authoring-rules.ts index 0486b052387..850487eb5b5 100644 --- a/packages/lint/src/authoring-rules.ts +++ b/packages/lint/src/authoring-rules.ts @@ -1176,27 +1176,18 @@ export const AUTHORING_RULES: readonly AuthoringRule[] = [ // while a schema error elsewhere in the stack cannot take these findings down // with it. // - // Advisory, deliberately, and this is the whole shape of #5068's first step: - // wiring the parse is the precondition for enforcement, not the enforcement - // (#5020, one surface over). The live corpus violates the declarations in two - // places that are open contract questions — inline i18n label maps on three - // published platform pages (#5728) and the record picker's declared-but-unread - // `displayField` (#5775) — so gating today would fail the platform's own pages - // to enforce declarations the platform does not keep. The error upgrade is a - // separate step, once the warning-period inventory is empty. - // - // #5775 settled the record picker's half: `displayField` is retired in favour - // of the `labelField` the renderer actually reads. Its claim that "the rest of - // the keys the renderers honour are declared" did NOT hold — #6776 found five - // more (`page:header` `recordChrome`/`showStar`/`showCopyId`, - // `page:accordion.variant`, and the tab strip's visual style, whose declared - // spelling `page:tabs.type` collided with the component node's own dispatch - // key and so was unauthorable in the flat and JSX carriers). All five are - // declared as of #6776, the last as the renamed `tabStyle`. What remains - // before the error upgrade is #5728 and two page rewrites. + // Gating since #23026, for ONE class: a `retiredKey()` tombstone's verdict is + // an `error`, so `os validate`, `os build` and `os lint` refuse a retired + // prop with its prescription. Every other finding stays a `warning`. #5068's + // first step was advisory on purpose — wiring the parse is the precondition + // for enforcement, not the enforcement (#5020, one surface over) — and the + // error upgrade was to be its own step once the warning-period inventory was + // empty. Its blockers (#5728, #5775, #6776's undeclared props, two page + // rewrites) are all closed, and the inventory measured 0. The rule's module + // header records the measurement and why the other classes stay advisory. { name: 'validateComponentProps', - tier: 'advisory', + tier: 'gating', input: 'normalized', commands: ALL, source: 'packages/lint/src/validate-component-props.ts', diff --git a/packages/lint/src/validate-component-props.test.ts b/packages/lint/src/validate-component-props.test.ts index 5ecf6ddf37c..6c29afcd19d 100644 --- a/packages/lint/src/validate-component-props.test.ts +++ b/packages/lint/src/validate-component-props.test.ts @@ -77,26 +77,26 @@ describe('validateComponentProps — undeclared keys', () => { expect(findings).toEqual([]); }); - // #20758 — `PageHeaderProps.breadcrumb` is a retiredKey tombstone. A page is - // never hard-refused for carrying it: this rule is the door where an author - // meets the prescription, and every finding it files is a WARNING. - it('reports a retired page-header `breadcrumb` as a warning carrying the prescription, for `true` and `false`', () => { + // #20758 — `PageHeaderProps.breadcrumb` is a retiredKey tombstone. This rule + // is the door where an author meets the prescription, and since #23026 a + // retired key is the one class it refuses: an ERROR, whatever the value. + it('refuses a retired page-header `breadcrumb` as an error carrying the prescription, for `true` and `false`', () => { for (const breadcrumb of [true, false]) { const findings = validateComponentProps( stackWith([{ type: 'page:header', properties: { title: 'T', breadcrumb } }]), ); expect(findings, `breadcrumb: ${breadcrumb}`).toHaveLength(1); const [f] = findings; - expect(f.severity).toBe('warning'); + expect(f.severity).toBe('error'); expect(f.path).toBe('pages[0].regions[0].components[0].properties.breadcrumb'); expect(f.message).toContain('`page:header` property `breadcrumb` was removed in @objectstack/spec 17'); } }); // #21589 — an `object-master-detail-form` detail entry's `sortField` is a - // retiredKey tombstone, one array level down. The same door, the same - // warning: the finding names the entry's key, and the page is never refused. - it('reports a retired detail-entry `sortField` as a warning carrying the prescription, at the entry\'s key', () => { + // retiredKey tombstone, one array level down. The same door and, since + // #23026, the same refusal: an error that names the entry's key. + it('refuses a retired detail-entry `sortField` as an error carrying the prescription, at the entry\'s key', () => { const findings = validateComponentProps( stackWith([ { @@ -113,7 +113,7 @@ describe('validateComponentProps — undeclared keys', () => { ); expect(findings).toHaveLength(1); const [f] = findings; - expect(f.severity).toBe('warning'); + expect(f.severity).toBe('error'); expect(f.rule).toBe(COMPONENT_PROPS_INVALID); expect(f.path).toBe('pages[0].regions[0].components[0].properties.details.1.sortField'); expect(f.message).toContain('`object-master-detail-form` property `details[].sortField` was removed in @objectstack/spec 17'); @@ -424,7 +424,7 @@ describe('validateComponentProps — value verdicts', () => { ); const retired = invalid(findings).filter((f) => f.path.endsWith('.displayField')); expect(retired).toHaveLength(1); - expect(retired[0].severity).toBe('warning'); + expect(retired[0].severity).toBe('error'); expect(retired[0].message).toMatch(/removed in @objectstack\/spec 17\.0\.0[\s\S]*`labelField`/); }); @@ -647,8 +647,8 @@ describe('validateComponentProps — object-* blocks are dispatched (#7751)', () expect(f.where).toBe('page "probe_page" · object-grid'); expect(f.message).toContain('`filters`'); expect(f.message).toContain('Did you mean `filters` → `filter`?'); - // Warning tier only — the error upgrade stays gated on the #5068 - // inventory; this card did not move that gate (ruling 2026-08-12). + // Warning tier only — an undeclared key is not the retired-key class + // #23026 moved to `error`. expect(invalid(findings)).toEqual([]); }); @@ -743,10 +743,10 @@ describe('validateComponentProps — reads the post-conversion (normalized) tier }); describe('validateComponentProps — wiring', () => { - it('is registered as an advisory rule on all three authoring commands', () => { + it('is registered as a gating rule on all three authoring commands (#23026: a retired key is an error)', () => { const entry = AUTHORING_RULES.find((r) => r.name === 'validateComponentProps'); expect(entry).toBeDefined(); - expect(entry?.tier).toBe('advisory'); + expect(entry?.tier).toBe('gating'); expect([...(entry?.commands ?? [])].sort()).toEqual(['build', 'lint', 'validate']); }); @@ -761,6 +761,144 @@ describe('validateComponentProps — wiring', () => { }); }); +/** + * #23026 — the error upgrade the module header planned, for ONE class: a value + * verdict a `retiredKey()` tombstone raises is an `error`; every other finding + * stays a `warning`. Asserted on rule id, severity and path — the prescription + * is asserted as CARRIED (read back from the spec's own tombstone), never as + * prose copied here. + */ +describe('validateComponentProps — a retired key is an error, nothing else is (#23026)', () => { + /** The prescription the spec's own tombstone raises for `type.key`, read from the schema. */ + const prescription = (type: string, key: string): string => { + const schema = (ComponentPropsMap as unknown as Record { error?: { issues: Array<{ path: PropertyKey[]; message: string }> } } }>)[type]; + const issue = schema.safeParse({ [key]: 'probe' }).error?.issues.find((i) => i.path.length === 1 && i.path[0] === key); + expect(issue, `${type}.${key} raises no issue of its own`).toBeDefined(); + return issue!.message; + }; + + it('refuses the card\'s specimen — `object-grid` `defaultFilters` — as an error carrying its prescription', () => { + const findings = validateComponentProps( + stackWith([{ + type: 'object-grid', + properties: { + objectName: 'task', + defaultFilters: [{ field: 'status', operator: 'equals', value: 'open' }], + }, + }]), + ); + expect(findings.map((f) => [f.rule, f.severity, f.path])).toEqual([ + [COMPONENT_PROPS_INVALID, 'error', 'pages[0].regions[0].components[0].properties.defaultFilters'], + ]); + expect(findings[0].message).toContain(prescription('object-grid', 'defaultFilters')); + }); + + /** + * Every top-level tombstone in the map, found from the SCHEMA GRAPH (a key + * whose schema is `optional` over `never`), not from the issue shape the + * rule reads — so a tombstone whose verdict stopped classifying reds here, + * whichever row it sits on. + */ + it('refuses every top-level tombstoned prop in ComponentPropsMap as an error, at its own key', () => { + type Node = { _zod: { def: { type: string; innerType?: Node } } }; + const tombstones = Object.entries(ComponentPropsMap).flatMap(([type, schema]) => { + const shape = (schema as unknown as { shape?: Record }).shape ?? {}; + return Object.entries(shape) + .filter(([, node]) => node._zod.def.type === 'optional' && node._zod.def.innerType?._zod.def.type === 'never') + .map(([key]) => [type, key] as const); + }); + // Anti-vacuity: the walk reaches the card's specimen and a fair number besides. + expect(tombstones).toContainEqual(['object-grid', 'defaultFilters']); + expect(tombstones.length).toBeGreaterThan(30); + const missed = tombstones.filter(([type, key]) => { + const findings = validateComponentProps(stackWith([{ type, dataSource: { object: 'task' }, properties: { [key]: 'probe' } }])); + const at = findings.filter((f) => f.path === `pages[0].regions[0].components[0].properties.${key}`); + return !(at.length === 1 && at[0].rule === COMPONENT_PROPS_INVALID && at[0].severity === 'error'); + }); + expect(missed).toEqual([]); + }); + + it('refuses a retired ELEMENT\'s bag (`user:profile`) as an error — the same tombstone shape, one grain wider', () => { + const findings = validateComponentProps(stackWith([{ type: 'user:profile', properties: {} }])); + expect(findings.map((f) => [f.rule, f.severity, f.path])).toEqual([ + [COMPONENT_PROPS_INVALID, 'error', 'pages[0].regions[0].components[0].properties'], + ]); + }); + + it('refuses a tombstone inside a union arm when that arm is the only one judging keys (`publicPicker`)', () => { + const findings = validateComponentProps( + stackWith([{ + type: 'object-form', + properties: { objectName: 'task', sections: [{ label: 'S', fields: ['name', { field: 'owner', publicPicker: true }] }] }, + }]), + ); + expect(findings.map((f) => [f.rule, f.severity, f.path])).toEqual([ + [COMPONENT_PROPS_INVALID, 'error', 'pages[0].regions[0].components[0].properties.sections.0.fields.1'], + ]); + }); + + it('leaves that union verdict a warning when the same arm also rejects a value for another reason', () => { + const findings = validateComponentProps( + stackWith([{ + type: 'object-form', + properties: { objectName: 'task', sections: [{ label: 'S', fields: [{ field: 'owner', publicPicker: true, colSpan: 'wide' }] }] }, + }]), + ); + expect(findings.map((f) => [f.rule, f.severity])).toEqual([[COMPONENT_PROPS_INVALID, 'warning']]); + }); + + it('classifies per issue: a retired key beside a wrongly-typed prop is one error and one warning', () => { + const findings = validateComponentProps( + stackWith([{ type: 'object-grid', properties: { objectName: 'task', defaultFilters: [], pageSize: 'ten' } }]), + ); + const base = 'pages[0].regions[0].components[0].properties'; + expect(findings.map((f) => [f.rule, f.severity, f.path])).toEqual([ + [COMPONENT_PROPS_INVALID, 'error', `${base}.defaultFilters`], + [COMPONENT_PROPS_INVALID, 'warning', `${base}.pageSize`], + ]); + }); + + // The two closed-set refusals that landed beside this change reach the rule + // as `component-props-invalid` too, and are not the retired-key class. + it('keeps a url action\'s refused `target` scheme a warning', () => { + const findings = validateComponentProps( + stackWith([{ type: 'element:button', properties: { label: 'Go', action: { type: 'url', target: 'javascript:alert(1)' } } }]), + ); + expect(findings.map((f) => [f.rule, f.severity, f.path])).toEqual([ + [COMPONENT_PROPS_INVALID, 'warning', 'pages[0].regions[0].components[0].properties.action.target'], + ]); + }); + + it('keeps a `record:details` section `count` whose heading binds no hole a warning', () => { + const findings = validateComponentProps( + stackWith([{ type: 'record:details', properties: { sections: [{ label: 'Tags', fields: ['tags'], count: { field: 'tags' } }] } }]), + ); + expect(findings.length).toBeGreaterThan(0); + expect(findings.every((f) => f.rule === COMPONENT_PROPS_INVALID && f.severity === 'warning')).toBe(true); + }); + + it('keeps an undeclared key and a missing `dataSource.object` binding warnings', () => { + const findings = validateComponentProps( + stackWith([ + { type: 'page:header', properties: { title: 'T', titel: 'typo' } }, + { type: 'element:number', properties: { aggregate: 'count' } }, + ]), + ); + expect(findings.map((f) => [f.rule, f.severity])).toEqual([ + [COMPONENT_PROPS_UNKNOWN_KEY, 'warning'], + [COMPONENT_PROPS_INVALID, 'warning'], + ]); + }); + + it('reaches the shared pipeline as an error — the run `os validate` / `os build` / `os lint` refuse on', () => { + const findings = runAuthoringRules('validate', { + normalized: stackWith([{ type: 'object-grid', properties: { objectName: 'task', defaultFilters: [] } }]) as never, + }); + const mine = findings.filter((f) => f.rule.startsWith('component-props')); + expect(mine.map((f) => [f.rule, f.severity])).toEqual([[COMPONENT_PROPS_INVALID, 'error']]); + }); +}); + /** * #4001 batch A closed all 31 `ComponentPropsMap` shapes, including two UNION * ARMS. An arm's rejection travels a different road out of zod than an object's, diff --git a/packages/lint/src/validate-component-props.ts b/packages/lint/src/validate-component-props.ts index 5c1af337036..f986c5ad729 100644 --- a/packages/lint/src/validate-component-props.ts +++ b/packages/lint/src/validate-component-props.ts @@ -48,38 +48,59 @@ * author's view — and without this file needing to know which posture the spec * is at. * - * ## Why WARNING, and only warning, in this PR + * ## Severity — a RETIRED key is an `error`, every other finding a `warning` * - * Wiring the parse is the precondition for enforcement, not the enforcement - * (the #5020 lesson, one surface over). The corpus this landed on carries live - * violations that are open contract questions, not authoring mistakes: - * `I18nLabelSchema` is a plain `z.string()` while three published platform - * pages author inline `{ en, 'zh-CN', … }` maps that objectui resolves - * (#5728), and the record picker declared a required `displayField` that no - * renderer reads while honouring an undeclared `labelField` (#5775). Gating - * those would fail the platform's own pages to enforce declarations the - * platform does not itself keep. So every finding here is advisory, the - * warning-period inventory is the acceptance baseline for the error upgrade, - * and the upgrade is its own step once the inventory is empty. + * Wiring the parse was the precondition for enforcement, not the enforcement + * (the #5020 lesson, one surface over), so this rule landed advisory: the + * corpus it landed on carried live violations that were open contract + * questions, not authoring mistakes — inline `{ en, 'zh-CN', … }` label maps + * against a plain-string `I18nLabelSchema` on three platform pages (#5728), the + * record picker's required-but-unread `displayField` (#5775), five props the + * renderers read and the map did not declare (#6776), and two page rewrites + * (`page:card.visible` → `visibleWhen`; #5776's tab `key` → `value`). Gating + * then would have failed the platform's own pages to enforce declarations the + * platform did not keep. The plan was recorded here: the warning-period + * inventory is the acceptance baseline for the error upgrade, and the upgrade + * is its own step once the inventory is empty. Every blocker above is closed. * - * #5775 settled most of the spec side of that inventory — four keys nothing read - * are tombstoned with ADR-0087 conversions. Its stronger claim, that every key - * the renderers honour is now declared, was **incomplete**: #6776 re-counted - * against objectui `origin/main` and found five more author-facing props the - * renderers read and this map did not declare — `page:header`'s - * `recordChrome` / `showStar` / `showCopyId`, `page:accordion.variant`, and the - * tab strip's visual style, which the spec did declare but under a spelling - * (`page:tabs.type`) that collides with a page component's own dispatch key and - * is therefore unauthorable in the flat and JSX carriers. #6776 declares the - * four and renames the fifth to the `tabStyle` every carrier can express. Both - * cards are named here rather than only in a changelog because THIS comment is - * the inventory the error upgrade is measured against, and a count that reports - * itself complete while it is not is how the second half went unnoticed for a - * release. + * #23026 took that step for ONE class: a value verdict raised by a + * `retiredKey()` tombstone (`shared/retired-key.ts`, `@objectstack/spec`). A + * tombstone keeps a key declared only so that writing it raises the + * prescription, so reporting it as advice had `os validate` print + * "✓ Validation passed" over a key the ADR-0087 conversion registry tells + * authors is refused. The inventory, measured before the upgrade at + * objectstack `21c86444e`: `os validate` over `examples/app-crm`, `app-todo`, + * `app-multi-package` and `app-showcase`, and this rule over the five shipped + * pages outside them (`platform-objects` ×3, `plugin-approvals`, + * `qa/downstream-contract`); `packages/apps/*` and the `create-objectstack` + * template author no page. Retired-key findings: 0. Every other finding of + * this rule: 0 as well. * - * What stands between this rule and `error` is now #5728 and two page rewrites - * (`page:card.visible` → the ADR-0089 component-level `visibleWhen`; #5776's tab - * `key` → `value`), not the props map. + * The class is read off the ISSUE, never off its text ({@link isRetiredKeyVerdict}): + * + * - the tombstone's own issue, `invalid_type` expecting `never` — the shape a + * `retiredKey()` raises, and the discriminator the spec's own flow-node + * refusals use. A retired ELEMENT's row (`retiredComponentProps` in + * `ui/component.zod.ts`: `z.never` over the whole bag, with the element's + * prescription) raises the same shape and is the same class; + * - inside a collapsed `invalid_union`, when exactly one arm raised tombstone + * issues and nothing else — the one-arm rule of + * {@link unrecognizedKeysFromUnionArm}, for the same reason. It is live on + * `object-form` / `object-master-detail-form` `sections[].fields[]`, whose + * object arm tombstones `publicPicker`. + * + * What stays a `warning`, deliberately: a value rejected for any other reason + * (a wrong type, a missing required prop, a closed-set refusal such as a url + * action's `target` scheme or a `record:details` section's `count`), a + * retired enum VALUE (`enumWithRetiredValues` raises an enum issue, not a + * tombstone), an undeclared key, and the missing `dataSource.object` binding. + * Their inventory above (0) is the next step's baseline; moving them is a + * decision of its own, not a rider on this one. + * + * Where it refuses: the three CLI doors — `os validate`, `os build`, `os lint` + * — run every gating rule. The runtime publish door (the Studio, REST and MCP + * write path) does not run this rule (`surfaces` in `authoring-rules.ts`), so a + * page saved there is not refused for a retired key. * * ## Unregistered types are SKIPPED — a required semantic, not leniency * @@ -115,12 +136,12 @@ export const COMPONENT_PROPS_UNKNOWN_KEY = 'component-props-unknown-key'; export const COMPONENT_PROPS_INVALID = 'component-props-invalid'; /** - * Advisory on every finding this rule emits — see the module header. The type - * is a single literal rather than a union so the tier claim in - * `authoring-rules.ts` is provable from this file's source, which is exactly - * what `authoring-rule-wiring.test.ts` reads it for. + * `error` for a retired key ({@link isRetiredKeyVerdict}), `warning` for every + * other finding — see the module header. Emitting `error` makes the rule + * `gating` in `authoring-rules.ts`, which `authoring-rule-wiring.test.ts` + * verifies against this file's source. */ -export type ComponentPropsSeverity = 'warning'; +export type ComponentPropsSeverity = 'error' | 'warning'; export interface ComponentPropsFinding { severity: ComponentPropsSeverity; @@ -226,6 +247,42 @@ function unrecognizedKeysFromUnionArm(issue: LintZodIssue): LintZodIssue | undef return found; } +/** + * A `retiredKey()` tombstone's own issue: `z.never` refuses every value as + * `invalid_type` expecting `never`, carrying the prescription as its message. + * `expected` is read structurally, as the spec's flow-node refusals read it, + * because {@link LintZodIssue} declares only the fields the renderer needs. + */ +function isTombstoneIssue(issue: LintZodIssue): boolean { + return issue.code === 'invalid_type' && (issue as { readonly expected?: unknown }).expected === 'never'; +} + +/** + * Is this value verdict a RETIRED key the author wrote — the class this rule + * refuses as an `error` (module header, "Severity")? + * + * Directly, when the issue is the tombstone's own. Inside a collapsed + * `invalid_union`, only under the one-arm rule {@link unrecognizedKeysFromUnionArm} + * states: exactly one arm raised tombstone issues, and that arm raised nothing + * else. Two arms each refusing a retired key, or one arm refusing a retired key + * beside a value it rejects for another reason, do not say which shape the + * author meant, so the verdict stays the ordinary `warning` — `describeIssue` + * still prints every arm's message, the prescription included. + */ +function isRetiredKeyVerdict(issue: LintZodIssue): boolean { + if (isTombstoneIssue(issue)) return true; + if (issue.code !== 'invalid_union') return false; + const arms = issue.errors; + if (!arms || arms.length === 0) return false; + let found = false; + for (const arm of arms) { + if (!arm.some(isRetiredKeyVerdict)) continue; + if (found || !arm.every(isRetiredKeyVerdict)) return false; + found = true; + } + return found; +} + export function validateComponentProps(stack: AnyRec): ComponentPropsFinding[] { const findings: ComponentPropsFinding[] = []; if (!isRec(stack)) return findings; @@ -349,6 +406,24 @@ export function validateComponentProps(stack: AnyRec): ComponentPropsFinding[] { } continue; } + const subject = `${at.slice(base.length + 1) || 'properties'}: ${describeIssue(issue, props)}`; + // A retired key is refused; it is the one `error` this rule files. Its + // fix is the prescription the message already carries, so the hint + // never offers the other branch below — re-declaring a retired key in + // the schema would undo its retirement. + if (isRetiredKeyVerdict(issue)) { + findings.push({ + severity: 'error', + rule: COMPONENT_PROPS_INVALID, + where, + path: at, + message: `${subject} — a retired key is refused, so this stack does not pass until it is gone`, + hint: + `Apply the prescription above: \`${type}\`'s props schema keeps a retired key only to refuse ` + + 'it, so the fix is the rename or removal it names, never a schema change.', + }); + continue; + } findings.push({ severity: 'warning', rule: COMPONENT_PROPS_INVALID, @@ -358,9 +433,7 @@ export function validateComponentProps(stack: AnyRec): ComponentPropsFinding[] { // advisory posture it used to explain (judged here as a warning, and // the props bag is not parsed on the storage path) is the verdict's // consequence and rides the message. - message: - `${at.slice(base.length + 1) || 'properties'}: ${describeIssue(issue, props)} — ` + - 'nothing refuses it today, so the renderer receives the props as written', + message: `${subject} — nothing refuses it today, so the renderer receives the props as written`, hint: `Give \`${type}\` props its schema accepts (ComponentPropsMap, @objectstack/spec/ui) — the ` + 'rejection above names what it expects — or, if the component really does honour what is ' + diff --git a/packages/lint/src/validate-page-field-bindings.ts b/packages/lint/src/validate-page-field-bindings.ts index ad2e6568a35..d6448e1b5e3 100644 --- a/packages/lint/src/validate-page-field-bindings.ts +++ b/packages/lint/src/validate-page-field-bindings.ts @@ -230,11 +230,13 @@ export const COMPONENT_FIELD_SPECS: Readonly> // NAME against the object) is not the question a retired key raises (commit cd584d559). // // A non-conformant page that writes one anyway is not left unattended: the - // #5068 props gate reports the key with its rename/delete prescription. That - // gate is advisory and CLI-only and lives in a different registry - // (`authoring-rules`) from this suite, so it neither precedes nor suppresses - // this rule — what these two entries actually added was a SECOND finding, - // saying a field named by a key that no longer exists does not exist either. + // #5068 props gate refuses the key as an `error` at `os validate` / + // `os build` / `os lint`, with its rename/delete prescription (it stays + // advisory for every other finding). That gate is CLI-only and lives in a + // different registry (`authoring-rules`) from this suite, so it neither + // precedes nor suppresses this rule — what these two entries actually added + // was a SECOND finding, saying a field named by a key that no longer exists + // does not exist either. // The prescription is the useful half; this half was noise on top of it. 'element:record_picker': { props: ['labelField'] }, }; diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index 96d69e4b513..4f65f89a760 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -5133,8 +5133,10 @@ const pageHeaderSubtitleAlias: MetadataConversion = { * conversion can honour at a nested site, not only at region level. * * All three are **retired from the load path**: each key is tombstoned in - * `ui/component.zod.ts`, so the loader rejects it loudly with the prescription - * and only `os migrate meta` lists the mechanical edits for existing sources. + * `ui/component.zod.ts`, so `os validate`, `os build` and `os lint` refuse it + * with the prescription (the component-props gate, `@objectstack/lint`; the + * load path keeps the open `properties` bag as written) and only + * `os migrate meta` lists the mechanical edits for existing sources. */ const RECORD_PICKER_COMPONENT_TYPE = 'element:record_picker'; @@ -5832,7 +5834,9 @@ const inlineActionApiParamsToBodyExtra: MetadataConversion = { * component's inner `type` (an action's `type: 'url'`, a tab item's fields) * into a rewrite target: only `properties.type` on a node whose own `type` is * `page:tabs` moves. The tombstone still carries the refusal at the authoring - * site (`tsc`) and at load (the parse), whatever the walk reached. + * site (`tsc`) and at `os validate` / `os build` / `os lint`, where the + * component-props gate refuses it as an `error` (the load path keeps the open + * `properties` bag as written), whatever the walk reached. * * `retiredFromLoadPath: true`: no alias window, deliberately. The tombstone owns * the refusal; this entry exists so `spec-changes.json`, the upgrade guide and @@ -8025,8 +8029,12 @@ const translationPerAppSettingsRemoved: MetadataConversion = { * Reach: the literal defaults the narrowed gates judge — a `time` field's * (`objects[]`, `objectExtensions[]`) and an action param typed `time` * (`actions[]`, `objects[].actions[]`, an `element:button`'s inline action). - * Retired from the load path: an author is refused at parse, and data at rest - * and `os migrate meta` replay it. + * Retired from the load path: an author is refused at parse — except on an + * `element:button`'s inline action, which sits in the open `properties` bag the + * load path keeps as written, where the refusal is an advisory + * `component-props-invalid` finding at `os validate` / `os build` / `os lint` + * (the component-props gate, `@objectstack/lint`) — and data at rest and + * `os migrate meta` replay it. */ const timeDefaultUtcSuffixDropped: MetadataConversion = { id: 'time-default-utc-suffix-dropped', @@ -8322,8 +8330,12 @@ const translationComponentSubmitLabelRemoved: MetadataConversion = { * positions the component-props gate judges. `variant` on any other component * type is that component's own vocabulary and is never read here. * - * `retiredFromLoadPath`: an author is refused at parse with the prescription - * rather than silently rewritten; stored rows replay it at rehydration + * `retiredFromLoadPath`: an author is not silently rewritten — `os validate`, + * `os build` and `os lint` report an old spelling as an advisory + * `component-props-invalid` finding carrying the prescription (the + * component-props gate, `@objectstack/lint`: a retired enum value is a value + * verdict, a warning; the load path keeps the open `properties` bag as + * written); stored rows replay it at rehydration * (`applyConversionsToStoredItem`) and `os migrate meta` lists the edit for * existing sources. Idempotent by construction: the rewrite's output is * outside its own input set. @@ -9818,7 +9830,8 @@ function gridFilterHasContent(value: unknown): boolean { * Zero authored occurrences in this repository (the showcase pins it at zero), * so the entry exists for stored `sys_metadata` rows and for authors outside * the repo. Retired from the load path: an author writing the key is refused - * at the parse with the prescription. + * by `os validate`, `os build` and `os lint` (the component-props gate) with + * the prescription. */ const objectGridDefaultFiltersRemoved: MetadataConversion = { id: 'object-grid-default-filters-removed', @@ -10606,8 +10619,10 @@ const objectKanbanQuickAddRemoved: MetadataConversion = { * nothing fills the slot. The console draws the navigation trail once, in the * shell (`@object-ui/app-shell` `AppHeader`, inside `/apps/:appName/*`), so the * retirement takes the key away rather than building a second trail. The - * tombstone on `PageHeaderProps` refuses it for a live author (advisory, via - * the props lint: `PageComponentSchema.properties` is an open bag). + * tombstone on `PageHeaderProps` refuses it for a live author: `os validate`, + * `os build` and `os lint` refuse it as an `error` with the prescription (the + * component-props gate, `@objectstack/lint`; the load path keeps the open + * `properties` bag as written). * * **A delete of both values.** `true` and `false` go alike: neither ever drew * a trail, so there is no value to preserve and no rewrite target. The one @@ -13548,8 +13563,10 @@ const dashboardWidgetChartConfigStructureRemoved: MetadataConversion = { * `sort_field` (`:874`). An authored value changed nothing on that console, so * deleting it preserves observed behaviour exactly; the line order is kept by * the child object's own field, which the entry's tombstone names. The - * tombstone refuses the key for a live author (advisory, via the props lint: - * `PageComponentSchema.properties` is an open bag). + * tombstone refuses the key for a live author: `os validate`, `os build` and + * `os lint` refuse it as an `error` with the prescription (the component-props + * gate, `@objectstack/lint`; the load path keeps the open `properties` bag as + * written). * * ⚠️ Scoped by component `type` and by POSITION — `properties.details[]` of an * `object-master-detail-form` — never by key name: `sortField` is an ordinary diff --git a/packages/spec/src/migrations/entries/retired-keys/18.ui__PageHeaderProps__breadcrumb.ts b/packages/spec/src/migrations/entries/retired-keys/18.ui__PageHeaderProps__breadcrumb.ts index 553e2574b5e..3fc620b9626 100644 --- a/packages/spec/src/migrations/entries/retired-keys/18.ui__PageHeaderProps__breadcrumb.ts +++ b/packages/spec/src/migrations/entries/retired-keys/18.ui__PageHeaderProps__breadcrumb.ts @@ -14,8 +14,9 @@ // ⭐ RETIRED-DEFAULT RESIDUE: not owed, although the key carried // `.default(true)`. The default was never materialized: a page parses its // component `properties` as an open bag, and this row is parsed only by the -// advisory props lint, which writes nothing back — so no released toolchain -// emitted `breadcrumb: true` into an artifact nobody authored. +// props lint (advisory in every release that shipped the default), which +// writes nothing back — so no released toolchain emitted `breadcrumb: true` +// into an artifact nobody authored. // // Registered under 18, not 17: v17.0.0 was cut before this landed, so the // removal ships on the 17.x line (launch-window convention: accept-set diff --git a/packages/spec/src/ui/component.zod.ts b/packages/spec/src/ui/component.zod.ts index a38da962ff9..12adf3a7859 100644 --- a/packages/spec/src/ui/component.zod.ts +++ b/packages/spec/src/ui/component.zod.ts @@ -281,10 +281,10 @@ import { INLINE_GRID_SORT_FIELD_LIST } from '../data/inline-grid-sort-fields'; // `saveMetaItem` / REST `/meta` write still stores an unvalidated props bag // (#4463's fourth wall). That is recorded, not fixed, by #5068. // -// The gate is WARNING-level in this first step. The live corpus violated these +// The gate was WARNING-level in this first step. The live corpus violated these // declarations in places that were open contract questions rather than -// authoring mistakes, and the inventory is the acceptance baseline for the -// error upgrade. Two of the three entries are now cleared: +// authoring mistakes, and the inventory was the acceptance baseline for the +// error upgrade. Two of the three entries were cleared here: // // - #5775 declared the keys objectui's renderers honour and tombstoned the // four nothing read. @@ -296,9 +296,11 @@ import { INLINE_GRID_SORT_FIELD_LIST } from '../data/inline-grid-sort-fields'; // same ruling and moved onto it. That retired all 42 `component-props-invalid` // findings this gate reported on the platform pages (34 label + 8 content). // -// What remains before the upgrade to error is the page rewrites -// (`page:card.visible` → the component-level `visibleWhen`, #5776's tab `key` -// → `value`), not a declaration in this file. +// The page rewrites (`page:card.visible` → the component-level `visibleWhen`, +// #5776's tab `key` → `value`) closed the third, and #23026 took the upgrade +// for one class once the inventory measured empty: a `retiredKey()` +// tombstone's verdict is an `error` at `os validate` / `os build` / `os lint`. +// Every other finding of the gate stays a warning (#23147 carries the rest). // // ⚠️ One inventory item batch A ADDED rather than closed, because measuring the // renderers turned it up: objectui's Studio block designer publishes inputs that @@ -651,8 +653,9 @@ export const PageHeaderProps = strictObject({ * inside `/apps/:appName/*`), so a page-level renderer would duplicate it * rather than supply something missing. The default `true` was never * materialized into a built artifact: `PageComponentSchema.properties` is an - * open bag, and this row is parsed only by the advisory props lint, which - * writes nothing back — so no retired-default residue stage is owed. + * open bag, and this row is parsed only by the props lint (advisory in every + * release that shipped the default), which writes nothing back — so no + * retired-default residue stage is owed. * * Stored and built pages that carry the key (`true` or `false`) are stripped * by the D2 conversion `page-header-breadcrumb-removed`, with a notice. @@ -4844,8 +4847,11 @@ export type ElementRepeaterPropsParsed = z.infer { }); it('never hard-refuses an existing page: the page parse still accepts a node carrying a flat key', () => { - // The open `properties` bag is the props lint's to judge (advisory), not the page parse's. + // The open `properties` bag is the props lint's to judge (a retired flat key is an `error` + // at `os validate` / `os build` / `os lint`), not the page parse's. const r = PageComponentSchema.safeParse({ type: 'element:repeater', properties: { object: 'deal' } }); expect(r.success).toBe(true); }); diff --git a/packages/spec/src/ui/form-field-public-picker-retirement.test.ts b/packages/spec/src/ui/form-field-public-picker-retirement.test.ts index e4bb7fb1f11..a947f40e1f1 100644 --- a/packages/spec/src/ui/form-field-public-picker-retirement.test.ts +++ b/packages/spec/src/ui/form-field-public-picker-retirement.test.ts @@ -315,6 +315,10 @@ describe('tree-scoped absence: no form field inside the declared radius still au // The REST strip pin authors it on a STORED pre-retirement row, the exact // row the old opt-in would have kept, to prove the strip is unconditional. 'packages/rest/src/public-form-routes.test.ts', + // The props-lint pin authors it on an `object-form` field to prove the + // component-props gate refuses a tombstone inside a union arm as an error + // at `os validate` / `os build` / `os lint`. + 'packages/lint/src/validate-component-props.test.ts', // This pin names the key to assert its absence. THIS_FILE, ]); diff --git a/packages/spec/src/ui/master-detail-detail-sort-field-retirement.test.ts b/packages/spec/src/ui/master-detail-detail-sort-field-retirement.test.ts index b55f882800a..36274034120 100644 --- a/packages/spec/src/ui/master-detail-detail-sort-field-retirement.test.ts +++ b/packages/spec/src/ui/master-detail-detail-sort-field-retirement.test.ts @@ -19,8 +19,9 @@ * carries the prescription, which names the derived field set from its * one declaration (`data/inline-grid-sort-fields.ts`), and the key's * input type is `never`. `PageComponentSchema.properties` is an open bag, - * so the props schema is reached by the advisory props lint and never by - * the page parse: an existing page is never hard-refused. + * so the props schema is reached by the props lint — which refuses this + * tombstone as an `error` at `os validate` / `os build` / `os lint` — and + * never by the page parse: an existing page is never hard-refused at load. * 2. D2 conversion `object-master-detail-form-detail-sort-field-removed` * (step 18), a strip scoped by component type and by position, retired * from the load path: a stored or built page replays clean, with a notice. @@ -443,7 +444,7 @@ describe('tree-scoped absence: nothing inside the declared radius still authors const EXCLUDED = new Set([ // This pin authors the key to assert its refusal and its conversion. THIS_FILE, - // The props-lint pin authors the key to assert the advisory warning an + // The props-lint pin authors the key to assert the refusal (an `error`) an // author meets at `os validate` / `os build` / `os lint`. 'packages/lint/src/validate-component-props.test.ts', ]); diff --git a/packages/spec/src/ui/page-header-breadcrumb-retirement.test.ts b/packages/spec/src/ui/page-header-breadcrumb-retirement.test.ts index 37fe2b3f473..dfcfac4f610 100644 --- a/packages/spec/src/ui/page-header-breadcrumb-retirement.test.ts +++ b/packages/spec/src/ui/page-header-breadcrumb-retirement.test.ts @@ -16,8 +16,9 @@ * 1. A `retiredKey()` tombstone on `PageHeaderProps`, a `strictObject` — the * refusal carries the prescription, and the key's input type is `never`. * `PageComponentSchema.properties` is an open bag, so the props schema is - * reached by the advisory props lint and never by the page parse: an - * existing page is never hard-refused. + * reached by the props lint — which refuses this tombstone as an `error` + * at `os validate` / `os build` / `os lint` — and never by the page parse: + * an existing page is never hard-refused at load. * 2. D2 conversion `page-header-breadcrumb-removed` (step 18), a strip of * both values from every `page:header`, retired from the load path: a * stored or built page replays clean, with a notice. @@ -352,7 +353,7 @@ describe('tree-scoped absence: nothing inside the declared radius still authors const EXCLUDED = new Set([ // This pin authors the key to assert its refusal and its conversion. THIS_FILE, - // The props-lint pin authors the key to assert the advisory warning an + // The props-lint pin authors the key to assert the refusal (an `error`) an // author meets at `os validate` / `os build` / `os lint`. 'packages/lint/src/validate-component-props.test.ts', ]);